Created attachment 173266 [details] CalcDoc with DropDown fiels to demonstrate errors The UI-Vcl-plugin lib instdir/program/libvclplug_kf5lo.so has still quite some errors. To use, build with flags creating this lib (--enable-kf5). Start LO using "SAL_USE_VCLPLUGIN=kf5 instdir/program/soffice" in commandline (or set SAL_USE_VCLPLUGIN as usual). Errors seen (probably not complete list): To reproduce, open bugdoc (LO528_Filter+Tesla.ods), Use dropdown button at 1st field to open DropDown/PopUp then (1) Problems with using TAB key, no crash, but main win makes EMPTY REPAINT and stays that way. Needs handish action to show UI && EditView again at all (e.g. resize) (2) No error with clicking at object selection CheckBoxes, but action is on MouseDown instead of MouseUp, also on leaving CheckBox area with still pressed MouseButton -> not standard, even the other 'normal' buttons trigger on MouseUp (trigger on LeaveArea is BAD since often used by user to terminate unwanted data entry without change) (3) DropDown/PopUp stays open and in FG when changing app on Linux desktop (keyboard, ALT+TAB) -> very strange, unwanted effects -> e.g. gtk3_kde5 avoids that by simply FORBIDDING app/focus change wile popup is open
(In reply to Armin Le Grand from comment #0) > Errors seen (probably not complete list): > To reproduce, open bugdoc (LO528_Filter+Tesla.ods), Use dropdown button at > 1st field to open DropDown/PopUp then > > (1) Problems with using TAB key, no crash, but main win makes EMPTY REPAINT > and stays that way. Needs handish action to show UI && EditView again at all > (e.g. resize) This sounds like the same issue issue as bug 143114, which I encountered just today. I just double-checked it's a regression from the same commit. > (2) No error with clicking at object selection CheckBoxes, but action is on > MouseDown instead of MouseUp, also on leaving CheckBox area with still > pressed MouseButton -> not standard, even the other 'normal' buttons trigger > on MouseUp (trigger on LeaveArea is BAD since often used by user to > terminate unwanted data entry without change) That behaves the same with gen VCL plugin and on Win, so might not be specific to qt5/kf5, but an issue with the general VCL implementation (as opposed to gtk3, which might use native controls)? > (3) DropDown/PopUp stays open and in FG when changing app on Linux desktop > (keyboard, ALT+TAB) -> very strange, unwanted effects -> e.g. gtk3_kde5 > avoids that by simply FORBIDDING app/focus change wile popup is open Same with "gen" in my tests, while gtk3 behaves better. Can we see (1) as a duplicate of bug 143114? Are (2) and (3) the same thing or does it possibly make sense to open separate bug reports for those? (unless you plan to fix them anyway ;-))
(In reply to Michael Weghorn from comment #1) > (In reply to Armin Le Grand from comment #0) > > Errors seen (probably not complete list): > > To reproduce, open bugdoc (LO528_Filter+Tesla.ods), Use dropdown button at > > 1st field to open DropDown/PopUp then > > > > (1) Problems with using TAB key, no crash, but main win makes EMPTY REPAINT > > and stays that way. Needs handish action to show UI && EditView again at all > > (e.g. resize) > > This sounds like the same issue issue as bug 143114, which I encountered > just today. I just double-checked it's a regression from the same commit. Jan-Marek's pending fix for tdf#143334 pending at https://gerrit.libreoffice.org/c/core/+/119237 actually fixes this aspect for me.
(1) should be fixed by the fix for tdf#143334 (3) should be fixed by commit 63f92f185b78db5a575da58efac3f94a8cb5a5f6 ("Qt5 fix Qt::Popup window handling") (2) is ... complex. It's not a normal checkbox problem. These work correct. The problem is a checkbox inside a treeview. If you have a treeview, you're actually handling rows. And if you hold the left button and move the mouse, you start a drag operation of the row. void SvTreeListBox::StartDrag( sal_Int8, const Point& rPosPixel ) { Point aEventPos( rPosPixel ); MouseEvent aMouseEvt( aEventPos, 1, MouseEventModifiers::SELECT, MOUSE_LEFT ); MouseButtonUp( aMouseEvt ); So now you know that actually when starting a drag you don't, because LO now generates a MouseButtonUp + MouseEventModifiers::SELECT event... And select means toggle for a checkbox, which is what you observe as wrong, when you start the drag. Then there is a lot of special handling code, when you actually start the drag operation on a checkbox (AKA SvLBoxItemType::Button), see SvImpLBox::ButtonDownCheckCtrl and friends. If you comment these three lines, you get a selection following your mouse. Maybe that is actually the correct behavior for a normal list view? AKA you just select on mouse up (but just if you don't start the drag by clicking on a button currently). But then it still toggles the checkbox for the last selected row, even when you release the mouse outside the widget. And what should happen, if you start the select on a row, but release on a 2nd? If you follow the "selection on up", that should select the current row. And it gets more interesting if you have rows with multiple buttons, like in Options >> Load/Save >> MS Office. There toggle on selection doesn't make any sense IMHO... P.S. and there was tdf#116675, the parent for a whole lot of regression and other "pleasantries", like https://gerrit.libreoffice.org/c/core/+/108947 (and instead of implementing a click handler for the autofilter list box, NIST changed the general widget...). Instead of having a special handler for row selection, which toggles the checkbox, we get a more complex general widget... Current conclusion: any fix on top of this can get messy with potential regressions.
My suggestion is to test (1) and (3), then close this and open a new report for (2). And we need something that defines the expected behavior, before we can even start implementing a "fix".
Looking for (2). As always, it's more complicated as thought, e.g. in standard mode gtk3 I checked for comparison a normal (as I thought) CheckBox in Tools/Options/UserData, e.g. "Use Data for Document Prioperties". That one can be interrupted in action after MouseDown to the left, top and bottom, but NOT to the right (?) - VERY strange. So the user who wants to interrupt his false-click has to be lucky to go to the correct direction to do so - also NO optical feedback while executing - that is usual to lead/support the user in what he is doing :-( This might hint to some weird leaving-follow-mousemovement area (despite MouseFollow should be active until MouseButtonUp). In that case the non-conform behaviour in the CheckBox in the DropDown may have to do with being restricted to all sides (?) And this is in standard gtk3 mode. Wanted to look at kf5-mode, though...
@Jan-Marek: Yes, SvTreeListBox::StartDrag gets the MouseButtonDown. In principle it's not complicated. If a TreeListEntry has a CheckBox and it gets hit, do NOT start the Drag as here, but forward that action to the CheckBox. I know, sounds simple, but is probably not. It depends if a ListBoxEntry has a 'real' CheckBox or only 'fakes' it. If it's a real one, action can be forwarded. If not, it would also have to be 'faked'. Pobably the rest of actions would also happen at SvTreeListBox, so - until action is finished - some 'forward' mode would be needed (or faked). In short: The hierarchy represented by SvTreeListEntry when containing active UI elements like CheckBoxes (seen in the code there might also be buttons?) HAS to be used/represented on MouseActions by checking if these are hit && forwarding the action. That would be the only clean solution, maybe including a 'forward' mode while mouse is captured (in this case by CheckBox). Need to find out if there exists a real CheckBox. Faking all that if not would mean to double the CheckBox behaviour locally - not effective and error-prone as soon as someone changes the CheckBox behaviour. Problem is this cannot really be debugged - argh - since this immedialely leads to deadlocks in the UI using gdb in any form in another window due to the mouse being locked :-(
It's an SvLBoxButton which is capable to show a CheckBox && a Button - sigh. In different UIs it might still be incarnated as 'real' Window/Button, so the question stays why it does not offer the normal UI's functionality to end a MouseButtonDown by leaving the area for MuseButtonUp...?
SvTreeListBox just has a list of ListBoxEntry(s) and these have SvLBoxButton(s) describing CheckBoxes/Buttons. I would say that MouseActions may/should/haveTo handle these if e.g. the CheckBox needs to be switched (it may work now due to 'something' doing this or being a real 'Window' in the resp. UI?). In consequence these would have to - check if SvLBoxButton is hit - start mouseLock, switch - during MouseMoves, may change state (pressed or not when outside) - at MouseButtonUp is the point to end mouseLock && execute action, NOT before Not sure how far that stuff interferes with the UI integrations of Qt and ssimilar stuff, but that's what *should* happen. I see that it's more easy to just execute (and then try to drag the ListEntry, BTW not needed when Button is hit)..?
Debugging this is not really possible, so I try to understand the code - SvTreeListBox is a nightmare, with it's base classes && massive implementations. I also used debug to see that the SvTreeListBox::StartDrag gets triggered by some Qt action, while e.g. MouseButtonDown is triggered regular by the office. Also we have Win/Linux/Mac. I just checked Linux where we have five UI possibilities (currently) selected by e.g. setting env SAL_USE_VCLPLUGIN: (a) "gen" (b) "gtk3_kde5" (c) "gtk3" (d) "qt5" (e) "kf5" To get all of these to experimant you need to build using --enable-kf5 --enable-qt5 --enable-gtk3-kde5 in autogen configuration. Versions (a..e) behave not the same, so the 1st problem would be to make them behave the same. BTW: A good example how a CheckButton *should* behave is in the BugDoc right above those lists when shown, the 'All' button - which works the same in (a..e). Checking why SvTreeListBox::StartDrag trggers a MouseButtonUp-event - maybe there are hints in the git history. Interestingly this is triggered by MouseMove, *not* from MouseButtonDown/Up (!)...
The three lines from SvTreeListBox::StartDrag originate from ac7acb0ab1329913b0cec79790adcde0263960be "Merged SvTreeListBox and SvLBox" from 2012-10-11 - pretty old. Thus it originates from "void SvLBox::StartDrag" and was merged-in to SvTreeListBox::StartDrag. No idea why it was like that in SvLBox, but it was *not* originally part of SvTreeListBox. This is like that since 2021, so I will not dig deeper - those three lines may not be needed, but no direct hints from the past :-( Basic problem stays that SvTreeListBox::StartDrag (triggered from a MouseMove, NOT from MouseButton) *always* executes that MouseButtonUp. This should not happen if SvImpLBox (the EntryList) is actively handling a ButtonAction with CaptureMouse() being active. That again is alredady done actively in SvImpLBox::ButtonDownCheckCtrl, SvImpLBox::MouseMoveCheckCtrl, SvImpLBox::ButtonUpCheckCtrl. Adding a possibility to *ask/check* for it from SvTreeListBox...
Really hard to debug/change stuff here, but I made some progress and using kf5 it's already pretty good. Thus I created https://gerrit.libreoffice.org/c/core/+/119644 as a suggestion/first step to enhance this situation. As described there, the behavior is also dependent on the UI implementation used under Linux (gen/gtk3_kde5/gtk3/qt5/kf5) which are all (unfortunately) behaving differently... :-(
Checking linux render backends (a..e): (a) "gen": works as expected. Exception: Start klick in text, move to other entry, MouseButtonUp switches other entry. This should not trigger/change anyting if start and end entry are different. Leaving box completely works as expected. (b) "gtk3_kde5": Switches on MouseDown (c) "gtk3": same as (b) (d) "qt5": same as (a), including exception (e) "kf5": same as (a), including exception So we really have two groups: (a,d,e): Remaining problem is ButtonUp on text, not CheckBox. This may be fixed, seems to be done in vcl code of SvTreeListBox (b,c): Switches on MouseDown, but is not handled by SvTreeListBox, so probably native widgets. I would guess to fix that, creation of that widgets would need/does set other flags as wanted (?) I have no idea where this happens or is done, or how/where to fix that though So I will try to expand my suggested fix to (a,d,e) and get it work with ButtonUp on text...
What a wild hunt using SAL_LOG, +WARN and stuff - also tried SAL_NO_MOUSEGRABS but did not work reliably. In the end found in SvTreeListBox::MouseButtonUp a fix tdf116675 that does the trick, but with wrong remembered SvTreeListEntry. Corrected that and it seems to work well - no longer switching other lines on MouseButtonUp. Preparing commit...
Armin Le Grand (Allotropia) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/ca7dab5d96e73b7b4b045e2460e0b2ee150757db tdf#143114 Avoid StartDrag on TreeListBox when CaptureOnButton It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/183f66f79b40b6159a0edcf1b49c3aa612c90525 Revert "tdf#143114 Avoid StartDrag on TreeListBox when CaptureOnButton" It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Let's remove target since I reverted the patch because of regression (see tdf#143749 and its dup)
@Julien, @Buojaga: Thanks for identifying and taking action on tdf#143749. I will take a look ASAP on why there gets a crash triggered now - which unfortunately did not happen @commit-time :-(
Could reproduce on kf5 (use SAL_USE_VCLPLUGIN=kf5) in draw/impress. Reason is that pImpl == nullptr in SvTreeListBox::MouseButtonDown while pImpl is accessed unprotected (no check). pImpl is nearly never checked for nullptr except e.g. SvTreeListBox::Invalidate. Gets set using SvTreeListBox::InitTreeView() and that is called from constructor (SvTreeListBox::SvTreeListBox), so it should be always set... Looking at caller ImplHandleMouseEvent shows that pChild and it's VclReferenceBase has mbDisposed==true, that explains stuff. Thus I suppose those calls to pChild in MouseNotifyEvent::MOUSEBUTTONDOWN in ImplHandleMouseEvent should not be done at all (?)...
Checked SvTreeListBox::MouseButtonDown, and indeed win gets disposed in call pImpl->MouseButtonDown( rMEvt ); so I guess Julien was already on a hot trace in https://bugs.documentfoundation.org/show_bug.cgi?id=143749#c4 2nd possibility would be to ask for isDisposed() after pImpl->MouseButtonDown call, should also work. Looks as if pImpl usages *should* be all checked when as it seems any call can dispose it, that && calls in the stack above. That this is not the case shows that this stuff always was fixed on try-and-error (aka no 'concept' to secure it). So I see no other choice as to do the same for now (?)
Since SvTreeListBox::MouseButtonUp also calls parent at end of function (pImpl->MouseButtonUp) I tend to do it equal in SvTreeListBox::MouseButtonDown...
Fix on https://gerrit.libreoffice.org/c/core/+/120593, waiting for comments.
Armin Le Grand (Allotropia) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/eaac26c886a793dfb6c619a32a9fdeef98af3f79 tdf#143114 Avoid StartDrag on TreeListBox when CaptureOnButton It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
@alg: Is there something still missing from your POV or can we close it?
(In reply to Jan-Marek Glogowski from comment #3) > (3) should be fixed by commit 63f92f185b78db5a575da58efac3f94a8cb5a5f6 ("Qt5 > fix Qt::Popup window handling") Note that that commit (and others in the series) has been reverted in 7-2 branch (but not on master so far) to fix the Wayland-specific tdf#144585, s. tdf#144585 and https://gerrit.libreoffice.org/c/core/+/122356 for more details. Interestingly, even with the commit being reverted (with current 7-2 status from bibisect repo), popups only remain open on X11, not in a Plasma Wayland session (at least in my tests on current Debian testing). I used the toolbar item to insert a table in Writer for testing.
This needs re-evaluation, as more stuff has been fixed in general in the Qt VCL plugin. The reported bugs were supposed to be fixed, but the implementation caused many more problems then expected. And nobody is currently assigned, so switch to NEW for the moment.
Dear Armin Le Grand, To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/ If the bug is present, please leave a comment that includes the information from Help - About LibreOffice. If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice. Please DO NOT Update the version field Reply via email (please reply directly on the bug tracker) Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to 'inherited from OOo'; 4b. If the bug was not present in 3.3 - add 'regression' to keyword Feel free to come ask questions or to say hello in our QA chat: https://web.libera.chat/?settings=#libreoffice-qa Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team MassPing-UntouchedBug
(In reply to Jan-Marek Glogowski from comment #25) > This needs re-evaluation, as more stuff has been fixed in general in the Qt > VCL plugin. The reported bugs were supposed to be fixed, but the > implementation caused many more problems then expected. And nobody is > currently assigned, so switch to NEW for the moment. Since this ticket mentions multiples things at the same time, let's follow the suggestion from comment 4 to have a clear scope: (In reply to Jan-Marek Glogowski from comment #4) > My suggestion is to test (1) and (3), then close this and open a new report > for (2). And we need something that defines the expected behavior, before we > can even start implementing a "fix". -> Closing this ticket as fixed. @Armin: Please open a separate ticket for (2) if this is still an issue.