octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #53537] main_window::focus_changed() routine s


From: Dan Sebald
Subject: [Octave-bug-tracker] [bug #53537] main_window::focus_changed() routine should be improved
Date: Sat, 31 Mar 2018 17:18:31 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

URL:
  <http://savannah.gnu.org/bugs/?53537>

                 Summary: main_window::focus_changed() routine should be
improved
                 Project: GNU Octave
            Submitted by: sebald
            Submitted on: Sat 31 Mar 2018 09:18:30 PM UTC
                Category: GUI
                Severity: 3 - Normal
                Priority: 5 - Normal
              Item Group: Other
                  Status: None
             Assigned to: None
         Originator Name: 
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
                 Release: dev
        Operating System: Any

    _______________________________________________________

Details:

I'm adding this bug report because of a bug I'm observing in the
patch/changeset here

https://savannah.gnu.org/bugs/?53276#comment62

in which a new drag-and-drop feature doesn't work quite right 100% of the
time.  The logic for the new feature is based upon focus, in particular when
the signal


        // signal to all dock widgets for updating the style
        emit active_dock_changed (m_active_dock, dock);


should be emitted.

This has made me look into the main_window::focus_changed() routine.  Parts of
this look like they could be done better.  This loop comes to mind:


    octave_dock_widget *dock = nullptr;
    QWidget *w_new = new_widget;  // get a copy of new focus widget
    QWidget *start = w_new;       // Save it as start of our search
    int count = 0;                // fallback to prevent endless loop

    QList<octave_dock_widget *> w_list = dock_widget_list ();

    while (w_new && w_new != m_main_tool_bar && count < 100)
      {
        // Go through all dock widgets and check whether the current widget
        // widget with focus is a child of one of it
        foreach (octave_dock_widget *w, w_list)
          {
            if (w->isAncestorOf (w_new))
              dock = w;
          }

        if (dock)
          break;

        // If not yet found (in case w_new is not a childs of its dock
widget),
        // test next widget in the focus chain
        w_new = qobject_cast<QWidget *> (w_new->previousInFocusChain ());

        // Measures preventing an endless loop
        if (w_new == start)
          break;  // We have arrived where we began ==> exit loop
        count++;  // Limited number of trials
      }


There are a few things:

1) This count < 100 construct is strange.  Am I following correctly that this
loop goes around and around the focus chain until it reaches 100?  Certainly
that can be done better.  Could there possibly be more than 100 objects if the
GUI gets really busy?

2) The previousInFocusChain may apply to tab order.  I think it is possible
for some objects to be focusable but not tab-focusable, i.e., focusPolicy()
setting.  So, generally speaking, this isn't a good approach.

3) The choice to exit the loop when w_new != m_main_tool_bar or w_new ==
start.  This implies a very specific ordering of where m_main_tool_bar and
w_new exist in the chain of focus, i.e., the start (or end, depending on how
one views it).  I wonder if it is possible that this loop could exit
prematurely.




    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?53537>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]