Bug 128748 - Endless loop with query parameter on wrong input
Summary: Endless loop with query parameter on wrong input
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.0.3 release
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:6.5.0 target:6.4.0.1 target:6....
Keywords: bibisectRequest, regression
Depends on:
Blocks:
 
Reported: 2019-11-12 14:05 UTC by Gerhard Schaber
Modified: 2019-11-27 13:03 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
Test db (3.06 KB, application/vnd.sun.xml.base)
2019-11-12 14:05 UTC, Gerhard Schaber
Details
Screenshots (55.52 KB, application/zip)
2019-11-19 16:24 UTC, Gerhard Schaber
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gerhard Schaber 2019-11-12 14:05:55 UTC
Created attachment 155749 [details]
Test db

When entering a wrong text in a date field for a query parameter, you might end up with an error dialog that you cannot get rid of, and you need to kill the LibreOffice process.

1. Load testparameter.odb
2. Double-click query1
3. Enter a wrong date format for from_date, such as "1..2", when the parameter dialog pops up
4. Click on the second parameter to_date
5. LO will complain that it cannot convert the text to a date, and when you try to cancel the dialog, it will keep popping up again and again. There is no way to get rid of it, unless one kills LO
Comment 1 Alex Thurgood 2019-11-12 14:33:41 UTC
No repro for me with 

Version: 6.4.0.0.alpha1+
Build ID: f7bc0204a80a4f078c63f93b9892904686ad5b36
CPU threads: 4; OS: Mac OS X 10.15.1; UI render: default; VCL: osx; 
Locale: fr-FR (fr_FR.UTF-8); UI-Language: en-US
Calc: threaded

I get the error message, then clicking either OK or Next moves on to next field, with the same error message, and then back to no message. If I click on Cancel, I get the error message only once, then an empty resultset window is displayed for the failed query.
Comment 2 Alex Thurgood 2019-11-12 14:35:47 UTC
No repro either with 

Version: 6.3.2.2
Build ID: 98b30e735bda24bc04ab42594c85f7fd8be07b9c
Threads CPU : 4; OS : Mac OS X 10.15.1; UI Render : par défaut; VCL: osx; 
Locale : fr-FR (fr_FR.UTF-8); Langue IHM : fr-FR
Calc: threaded
Comment 3 Alex Thurgood 2019-11-12 14:40:03 UTC
Hmm, actually I can reproduce this but ONLY as follows;

Click on the first date parameter, enter the incorrect string
Click on OK to remove message
Click on OK again to remove message

If you move the mouse anywhere else in the parameter query dialog OTHER than the main window area, then you can escape the dreaded message recall. If you move the mouse anywhere within that window space, then the message gets displayed over and over again.
Comment 4 Alex Thurgood 2019-11-12 14:40:45 UTC
This never used to behave like this to my knowledge (it's been a while)
Comment 5 Gerhard Schaber 2019-11-13 07:17:24 UTC
So far:
6.0.6 good
6.2.6 bad
Comment 6 Gerhard Schaber 2019-11-13 08:06:59 UTC
6.1.6.3 good
6.0.2.3 bad
Comment 7 Gerhard Schaber 2019-11-13 09:16:20 UTC
6.2.0.alpha1 needs 4 clicks to get rid of the dialog, but at least you can get rid of it
6.2.0.beta1 bad
(In reply to Gerhard Schaber from comment #6)
> 6.1.6.3 good
> 6.0.2.3 bad

I meant 6.2.0.3
Comment 8 Gerhard Schaber 2019-11-13 09:30:12 UTC
I am assuming bug 121743, but the bibisect setup is not complete.
Comment 9 Alex Thurgood 2019-11-13 11:39:20 UTC
(In reply to Gerhard Schaber from comment #8)
> I am assuming bug 121743, but the bibisect setup is not complete.

Indeed, looks like a prime candidate.

@Caolan : care to take a look ? Seems like the weld of the ParameterDialog has had another unintended side effect.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=f1f0045c74c930c02592db7dd6d7b782729f9ee9
Comment 10 Gerhard Schaber 2019-11-13 16:02:05 UTC
It seems that regression was introduced with a977098f59a4c931c6f8a1d423720e682f5ed047. The committer is the same, though.
Comment 12 Gerhard Schaber 2019-11-13 16:08:02 UTC
But even before that was a change that makes it necessary to click 4 times on OK for the dialog to give up, but at least you can get rid of it.
Comment 13 Commit Notification 2019-11-13 16:20:17 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e151c12a14c1a2536393c82fa6027f251be1a7d6

Resolves: tdf#128748 warning dialog appearing on focus change

It will be available in 6.4.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.
Comment 14 Caolán McNamara 2019-11-14 09:00:36 UTC
fixed in master, 6-3 backport will follow in a while
Comment 15 Commit Notification 2019-11-14 12:38:11 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/57a0136944de5c54fa697a20fcd873516f32b3ba

Resolves: tdf#128748 warning dialog appearing on focus change

It will be available in 6.4.0.1.

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.
Comment 16 Gerhard Schaber 2019-11-15 09:57:31 UTC
The fix does not really improve it a lot. Once you move the mouse over the parameter list of the dialog, the error shows up again, and depending on what you do, you manage to get into an endless loop again (e.g. after clearing the field and moving the mouse over the dialog). You are able to edit the field a bit in the first place, if you do not move the mouse into the parameter list, and if you enter a valid date, you can move the move into the dialog again afterwards. From a usability perspective, the current approach is not very satisfying. It would be great to have the old behavior again (like in 6.1)
Comment 17 Caolán McNamara 2019-11-15 11:23:43 UTC
I suspect the build tested doesn't have the fix in it, help about has a "build id" listed in it which you can copy and paste in here to test that theory
Comment 18 Gerhard Schaber 2019-11-18 07:21:44 UTC
Since it was committed to master, I a assuming this is 6.5.0, and I could only find newer Windows builds for 6.5.0.

Version: 6.5.0.0.alpha0+ (x64)
Build ID: 8c85782bbe46963e2be32c3cb406982f1790fc2f
CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
Locale: de-AT (de_AT); UI-Language: en-US
Calc: threaded

Version: 6.5.0.0.alpha0+ (x64)
Build ID: 314f15bff08b76bf96acf99141776ef64d2f1355
CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
Locale: de-AT (de_AT); UI-Language: en-US
Calc: threaded

1. Enter "1..2" as first parameter, stay with mouse in the parameter dialog
2. Click on the second parameter field
3. An error will pop up
4. Click OK in the error dialog (OK is a little outside of the parameter dialog)
5. The first parameter field is editable (red)
6.a. If you move into the parameter dialog again without fixing the parameter first, the error dialog pops up again and again
6.b. If you fix the parameter first, and then move with the mouse into the parameter dialog intending to click on the second parameter (but without actually clicking), the focus automatically changes back and forth between first and second parameter, depending on the position of the mouse. This is irritating.

I could not try 6.4.0.1, yet, because I could not find any Windows build.
Comment 19 Xisco Faulí 2019-11-19 11:47:12 UTC
Verified in

Version: 6.4.0.0.beta1+
Build ID: 1987c98926a85a483a32ea78e460e563a6ea4705
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Caolán, thanks for fixing this issue!
Comment 20 Xisco Faulí 2019-11-19 11:50:40 UTC
(In reply to Gerhard Schaber from comment #18)
> Since it was committed to master, I a assuming this is 6.5.0, and I could
> only find newer Windows builds for 6.5.0.
> 
> Version: 6.5.0.0.alpha0+ (x64)
> Build ID: 8c85782bbe46963e2be32c3cb406982f1790fc2f
> CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
> Locale: de-AT (de_AT); UI-Language: en-US
> Calc: threaded
> 
> Version: 6.5.0.0.alpha0+ (x64)
> Build ID: 314f15bff08b76bf96acf99141776ef64d2f1355
> CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
> Locale: de-AT (de_AT); UI-Language: en-US
> Calc: threaded
> 
> 1. Enter "1..2" as first parameter, stay with mouse in the parameter dialog
> 2. Click on the second parameter field
> 3. An error will pop up
> 4. Click OK in the error dialog (OK is a little outside of the parameter
> dialog)
> 5. The first parameter field is editable (red)
> 6.a. If you move into the parameter dialog again without fixing the
> parameter first, the error dialog pops up again and again
> 6.b. If you fix the parameter first, and then move with the mouse into the
> parameter dialog intending to click on the second parameter (but without
> actually clicking), the focus automatically changes back and forth between
> first and second parameter, depending on the position of the mouse. This is
> irritating.
> 
> I could not try 6.4.0.1, yet, because I could not find any Windows build.

I can't reproduce it in

Version: 6.4.0.0.beta1+
Build ID: 1987c98926a85a483a32ea78e460e563a6ea4705
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded
Comment 21 Commit Notification 2019-11-19 11:53:38 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/commit/91ed8daea449c6c33217d891f431dcebb8a4bd4e

Resolves: tdf#128748 warning dialog appearing on focus change

It will be available in 6.3.4.

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.
Comment 22 Gerhard Schaber 2019-11-19 15:56:09 UTC
Is it supposed to work with the following version?
Version: 6.4.0.0.beta1 (x64)
Build ID: 4d7e5b0c40ed843384704eca3ce21981d4e98920
CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
Locale: de-AT (de_AT); UI-Language: en-US
Calc: threaded

It is not. I am using Windows!
Comment 23 Xisco Faulí 2019-11-19 16:01:13 UTC
(In reply to Gerhard Schaber from comment #22)
> Is it supposed to work with the following version?
> Version: 6.4.0.0.beta1 (x64)
> Build ID: 4d7e5b0c40ed843384704eca3ce21981d4e98920
> CPU threads: 8; OS: Windows 10.0 Build 17134; UI render: default; VCL: win; 
> Locale: de-AT (de_AT); UI-Language: en-US
> Calc: threaded
> 
> It is not. I am using Windows!

Yes. Could you please record a screenshot of the problem you are seeing ?
Comment 24 Gerhard Schaber 2019-11-19 16:24:14 UTC
Created attachment 155946 [details]
Screenshots

param1...Entering a wrong value
param2...Error dialog pops up after clicking to the second parameter
param3...Selection changes to second parameter, the error dialog pops up 4 times in total
param4...After clicking OK for all 4 error dialogs, the selection is back in the first parameter, but the mouse is grabbed
param5...Once moving the mouse over the parameter dialog, the error pops up again 4 times in total

When keeping the mouse outside the parameter dialog in step param4, and entering a good value for the first parameter, the error will not pop up again, but the selection of the parameters changes depending on where I move the mouse (even without clicking).

Could you put the old behavior as in 6.1 in place, which has an expected behavior?
Comment 25 Gerhard Schaber 2019-11-20 11:46:20 UTC
I am taking the liberty to reopen this, since it is not nearly there where it was with 6.1 (even if the "endless loop" is kind of gone). Not for Windows that is.
Comment 26 Caolán McNamara 2019-11-20 21:28:34 UTC
hmm, works fine with the gtk one but there is difficulty with the gen one with the effort to override the selected row during or near the time of the row selection
Comment 27 Caolán McNamara 2019-11-21 11:01:11 UTC
In gen showing a dialog on the "click" in the list causes the mouse release not to be seen by the list so it sticks in "selection" mode causing new selection events on moving around the list when the dialog is dismissed, triggering new dialogs.

There's no magic bullet in 6.1, under gtk there's infinite dialogs already, under gen the list brokenly tracks the mouse after the dialog appears until the next mouse click.

Most practical thing to do is to drop the error dialog causing the problem and replace it with a tooltip
Comment 28 Commit Notification 2019-11-21 12:11:41 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a614ae36e8688f070819804dc25cbaa8bbddba8f

tdf#128748 replace warning dialog with tooltip

It will be available in 6.5.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.
Comment 29 Commit Notification 2019-11-21 13:05:51 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/b14119f3e8b46800772dc818b215b2b9f35b5c8e

tdf#128748 replace warning dialog with tooltip

It will be available in 6.4.0.1.

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.
Comment 30 Commit Notification 2019-11-22 15:13:09 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/commit/20fa796a88042e2d9d881c02f0aca5f461c63a80

tdf#128748 replace warning dialog with tooltip

It will be available in 6.3.5.

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.
Comment 31 Caolán McNamara 2019-11-22 15:15:51 UTC
no warning dialog, so no dialog related endless loop.
Comment 32 Gerhard Schaber 2019-11-25 07:11:36 UTC
Thanks. I will try that as soon as there is a Windows build.

I am still wondering why it worked with error dialog in the past without issues with the mouse release.
Comment 33 Gerhard Schaber 2019-11-25 07:33:04 UTC
And is it possible to get the current fix in 6.3.4 as well?
Comment 34 Caolán McNamara 2019-11-25 20:07:09 UTC
wrt why behaviour changed, probably mostly because the underlying widget used for such lists changed from a "ListBox" to a "SvTreeListBox". In the older case I see "ListBox" also brokenly tracks the mouse after the dialog appears until the next mouse click. In both new and old cases the MouseButtonUp after the dialog appears is not seen by the widget so broken things happen, just more severe in the SvTreeListBox case because it generated entry select events as the mouse passed over them after missing the MouseButtonUp which a "ListBox" didn't.

And as to why change from a ListBox to a SvTreeListBox, that's because ListBoxes in every other toolkit are just buttons with dropdown lists, while the LibreOffice ones are dual-mode of that type and a TreeView-alike mode. Doing away with the dual-mode and having a specific widget for each facilitates using native widgets instead of the LibreOffice ones.

wrt 6.3.4: https://gerrit.libreoffice.org/#/c/83645/ is now proposed for 6-3-4
Comment 35 Gerhard Schaber 2019-11-26 06:42:36 UTC
Thanks
Comment 36 Xisco Faulí 2019-11-27 13:01:50 UTC
Verified in

Version: 6.5.0.0.alpha0+
Build ID: 3a6f270edfffb97763927b2732feacedbdac1e80
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: x11; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Caolán, thanks for fixing this issue with a new approach!
Comment 37 Commit Notification 2019-11-27 13:03:48 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-3-4":

https://git.libreoffice.org/core/commit/2dc5f3c0f89dca55aa7b2dec7772cd535c3a74b2

tdf#128748 replace warning dialog with tooltip

It will be available in 6.3.4.

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.