Priority: P2: Important
Affects Version/s: 4.6.2, 4.7.0
Fix Version/s: 5.10.0 Beta 1
Component/s: Widgets: Widgets and Dialogs
Environment:tag_v4.7.0-beta1, but the same in 4.6.2 and newest git 4.7.0;
except for 3 lines, QWidget::setTabOrder() was not changed since the beginning of qt git more than a year ago, so the bug is there at least since then
In short, for complex widgets with focus proxy in some situations it is not possible to place them in in the focus chain in a predictable way. In particular when using even one such widget in a dialog in .ui file created in designer, the focus chain will be unpredictable even when one sets in designer the tab order for all widgets.
I have a custom widget which has at least two children widgets, one of which is focusProxy of the main custom widget. In the attached examples the custom widget class is called "CheckableSpinBox"; it inherits QFrame; it has one checkbox and one spinbox as children; spinbox is a focusProxy of the whole CheckableSpinBox, checkbox is initially hidden (except for example3). The intention is to manipulate and access from the outside world (i.e. the code using it, or from designer) only CheckableSpinBox (which forwards some signals, exposes values etc.) -> how CheckableSpinBox is build of a QCheckBox and QSpinBox should be internal and not visible to the outside world. According to docs of setFocusProxy() this is exactly the situation for which focus proxy is intended:
void QWidget::setFocusProxy ( QWidget * w ):
Some widgets can "have focus", but create a child widget, such as QLineEdit, to actually handle the focus.
(i.e. the most natural situation is when a focus proxy is a child of the widget).
PROBLEM: if the widget has other children besides the child who is its focus proxy, then:
From code (see example1 explained below ):
- it is impossible to set tab order in such dialog by rearranging CheckableSpinBoxes only;
- a workaround requires exposing internal child widgets and manipulating them explicitly; it is not possible to set consistent tab order by referring only to the compound parent (I have such working workaround for example 1 - I can attach it if you want but it's ugly, totally not maintainable and destroys completely the idea of encapsulating internal widgets in the black box of the compound! it is not an acceptable solution at all; plus - it does not help if one wants to use designer);
- on top of this in current version of Qt the behaviour is prone to the fact that QSpinBox internally creates a child QLineEdit (this is not the reason of the problem; all I'm saying is that in example 1 resulting tab order would be changed without any change in example code if Trolls decide to build QSpinBox in a different way, without internal line edit - see the analysis of output from example 1 below)
From designer (see example 2 explained below):
- it is possible to expose such custom widget to designer, to use it in .ui files, put in layouts, set properties in designer etc. (of course! this is the most standard example of a custom widget!);
- but it is not possible AT ALL to control/set in designer tab order in a dialog containing such widgets (I mean: you can set focus order, but what you will get will be different!)
I attach a screenshot in which you can see what tab order was set in designer and what (different!) tab order you get in fact in running dialog.
Note: for bigger projects it is really essential (esp. from maintainability point of view) to set up forms (including the tab focus order!) in designer.
Since I have an idea how this should be fixed (see attached patch) what I want to write is quite long. Therefore, I chose to split my report into 3 parts:
- this main bugreport just explains what goes wrong in QWidget::setTabOrder("first", "second"); then it describes the attached examples 1 and 2 which reproduce the problem; then I state what I think has to be decided to fix this properly;
- then, a separate comment will contain some remarks about "first" (explaining a bugfix of a minor bug shown in example 3);
- then, a second comment will contain some remarks about "second" (explaining a bugfix of the MAIN major BUG reported here).
Sorry for the length, I hope these details will be useful.
Just to extract the essence of the problem, the behaviour of setTabOrder is the following:
1. you call it with 2 arguments; setTabOrder("first", "second");
2. in simple case (when neither has a focus proxy), widget "second" is moved in the focus chain to the place just after "first";
3. if "first" has focus proxy then "second" is moved to the place just after the last child of "first" (instead of just after "first" itself);
4. if "second" has focus proxy then just this focus proxy is moved (instead of "second" itself).
The simplest example where one can see how this goes completely wrong is the following:
- we have a compound widget W with a number of children, one of them is the focus proxy of W; let's assume that this focus proxy is NOT the last child of W;
- W has initially some position in the focus chain, and is followed immediately by its children;
- now, imagine we move W to some other place in the focus chain (i.e. call setTabOrder(A, W) )
- because of the rule (4.) above, only focus proxy of W (one of W's children) is moved in the focus chain; the rest remains in the old place in the focus chain;
- now let's call setTabOrder(W, B)
one expects the tab order to be A => W => B. BUT: remember rule (3.) above? If W has focus proxy (as it does in this example) then the call "setTabOrder(W, B)" places B after the last child of W. But the last child of W was NOT moved in the focus chain when we called "setTabOrder(A, W)" (only focus proxy was moved). This means that B will be placed in the focus chain just after ... the OLD position of W!
Irrespective of what was the motivation behind rules 1.-4. (it's not completely off, there are just bugs in how things are in fact done - see below), the result is illogical, counter-intuitive, simply - BAD. This behaviour is a total nonsense. And this gave us a lot of headache, and our clients a lot of unnecessary mouse-clicking ('cause simple tabbing in logical order was not possible)...
Again, the main problem I think is that rule (4.) just moves the focus proxy, leaving the rest of the children in the old place in the focus chain. This may destroy internal logic of the widget (if tab order is important to it), but more importantly - may leave behind "the last child" => next calls to setTabOrder (together with rule (3.)) will create total mess.
To show the problem in practice, I created a simplified example (see attached "example1-without_designer"): checkablespinbox.* defines my simple, standard compound custom widget "CheckableSpinBox" (a QFrame with 2 children: a checkbox and a spinbox; this internal spinbox is focus proxy of the whole "CheckableSpinBox"). The rest of the code just uses "CheckableSpinBox":
- I create a dialog with four "CheckableSpinBoxes":
- if I don't do anything more the initial tab order is the order of creation of CheckableSpinBoxes:
(which in fact is "A and all its children" => "B and all its children" etc.)
- I call:
The expected order would be:
but from user point of view focus order is (just compile and run attached example1 => this is how ui will behave):
Note however, that the truth is even more complex than this apparent behaviour (see attached debug output from example 1 - I use here simplified example where checkboxes are hidden (and only QSpinBoxes which are focus proxies of CheckableSpinBox are shown); the full tab order after these 2 simple calls to setTabOrder is even greater mess if you choose to look at checkboxes too).
Second example of the same problem (see attached "example2-with_designer"):
(1) there are files as above; additionally:
(2) designer_plugin/ contains source of designer plugin which uses this custom widget
(3) test_dialog/test_dialog_base.ui is a form: if you compile the designer plugin (2) you can open this form in designer
(4) the rest of files in test_dialog/ contain a full simple program which uses this .ui file
- so, I have a designer plugin for this custom widget
- using designer, I create a dialog and add four checkable spinboxes, set their properties etc.
- then I set tab order in designer.
When I preview the dialog in designer the tab order is wrong ( see attached designer screen shot -> the order which you get in preview does not match what was set in designer)! The same wrong order as in the preview is there if I compile run the example program which uses this .ui file.
If you look at .uic/ui_test_dialog_base.h resulting from uic, it contains:
i.e. it is correct (matches the order set in designer [see screenshot] and if spinXXX were simple widgets this would set correct tab order). Again: the problem lies not in designer but in setTabOrder() in src/gui/kernel/qwidget.cpp (at lest from the first git version in April 2009 to the newest qt git).
Note: in this example I do NOT call setTabOrder myself at all. My class is very straightforward and is used in the most straightforward way. Tab order is set in designer in the most straightforward way for all widgets and ... the resulting order is not what is set in designer!
Note: It's really, really bad and important! For even a bit more complex dialogs in bigger projects the fact that you cannot set tab order in designer is catastrophic , esp. from maintenance point of view (try to go an change tab order of a complex dialog if it is set from code... ; and if this bug is not fixed in qt, you would in fact have to maintain manually tab order for all children of the compound widgets - the code would have to be much, much more complex than the four calls to setTabOrder above). Anyway, currently the resulting tab order is almost "random" from the point of view of a person who designs ui - it depends on order in which widgets are created!
So what's wrong with QWidget::setTabOrder(first,second)? As I wrote at the beginning, it is very straightforward if there is no focus proxy - it just moves a widget "second" to a different place (right after "first") in the focus chain. What goes wrong is when it tries to be smart when focusProxy is used. The key bug is in what happens about "second", but what happens about "first" is also not optimal, so let's look at both.
So in next separate comments I will describe how I think it should be fixed -> I first look at "first" then into "second". These next two comments also along the way describe the attached bugfix patch..
|For Gerrit Dashboard: QTBUG-10907|
|205266,6||Widgets: be able to tab to a widget that has a focus proxy||5.10||qt/qtbase||Status: MERGED||+2||0|
|205362,11||Widgets: change QWidget::setTabOrder to understand compound widgets||5.10||qt/qtbase||Status: MERGED||+2||0|
|237817,5||QWidget: fix setTabOrder for compound widgets||5.12||qt/qtbase||Status: MERGED||+2||0|
|264529,5||QWidget: fix setTabOrder for compound widgets||5.12||qt/qtbase||Status: MERGED||+2||0|