[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/
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Octave-bug-tracker] [bug #53537] main_window::focus_changed() routine should be improved,
Dan Sebald <=