[Octave-bug-tracker] [bug #53537] main_window::focus_changed() routine should be improved

[Dan Sebald]

Follow-up Comment #2, bug #53537 (project octave):

After having tested a few things, I actually think the whole
count-and-next-in-chain mechanism can be dropped all together.  I've printed
out count--it never goes above 0 no matter where I click or tab to.  For
example, the tables and lists QObjects have individual fields that can tabbed
to/from but those are not individual QWidgets that exist inside, say, the
bigger list.  Even if we imagine they were, I don't see the point because even
a small table of 21 x 21 would already surpass 100 by the
next-in-chain-try-again approach.

Plus, this is more of an organizational thing, i.e., the fact that we really
should give proper ancestry for every object rather than be setting parent=0
for anything but the intended top-level windows.  I guess what I'm saying is
that any scenario that the next-in-chain-try-again mechanism is meant to catch
really should be avoided in the first place with good organization.  If we
were to take that mechanism out and just leave the core


    // 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;
      }


things would be fine, and if a problem does arise just go and fix the ancestry
in a logical way.  The title bar highlight for any particular case is not a
severe bug if something is missed in the short term...and like I said, I've
yet to see a case where count goes above 0.

I would say there are a few things to consider here, though.  The main thing
is that the tab-chain doesn't seem real good as far as moving around, but I
think this sort of thing is too much work and low severity for 4.4.  For
example (and this is true of other windows that have lists or tables), the
variable editor will highlight/focus a variable_dock_widget, i.e., subpanel,
but when I press tab the chain goes inside the table.  From there, one is
stuck inside the table tabbing from one cell to the next.  I'd actually prefer
the outer level tab chain not go *inside* the table but instead jump to the
next variable_dock_widget, e.g., "var1" tabs to "var2" tabs to "var3", etc. 
Mouse clicking *into* the table would then change the tab action to tab around
within the table.  This should be achievable because there can be more than
one tab chain (see http://doc.qt.io/qt-5/qgraphicswidget.html#setTabOrder ). 
It would nice if various levels had their own tab chain.  Say

1st level tab chain: octave_dock_widget

2nd level tab chain: within the octave_dock_widget, doesn't go from one
octave_dock_widget to the next

3rd level tab chain: within items on octave_dock_widget, primarily defined by
the type of object, e.g., tab, list, qscintilla-editor, etc.

and then have shift-tab go a level deeper and cntrl-tab go a level higher.  Or
something like that.

The other item is I'd like you to take a look at this code:


    // if new dock has focus, emit signal and store active focus
    // except editor changes to a dialog (dock=0)
    if ((dock || m_active_dock != edit_dock_widget) && (dock !=
m_active_dock))
      {
        // signal to all dock widgets for updating the style
        emit active_dock_changed (m_active_dock, dock);

        QList<QDockWidget *> tabbed = tabifiedDockWidgets (dock);
        if (tabbed.contains (m_active_dock))
          dock->set_predecessor_widget (m_active_dock);

        if (edit_dock_widget == dock)
          emit editor_focus_changed (true);
        else if (edit_dock_widget == m_active_dock)
          emit editor_focus_changed (false);

        m_active_dock = dock;
      }


Perhaps the following scenario does not arise, but what if dock=0 but
m_active_dock != edit_dock_widget?  That is what if

(dock || m_active_dock != edit_dock_widget)

passes, but dock=0?  Will the following crash if dock=0?


        QList<QDockWidget *> tabbed = tabifiedDockWidgets (dock);


or will it return a nullptr?

    _______________________________________________________

Reply to this item at:

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

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