1544167 - End drags when creating new dialog windows (e.g. download dialog or desktop notifications on Windows)

Status: RESOLVED FIXED

(Core :: Window Management, defect, P3)

Description

[Sam Foster [:sfoster] (he/him)]

See bug 1538460 and https://bugzilla.mozilla.org/show_bug.cgi?id=1538460#c17

On windows, when a modal dialog is shown while a drag is in progress, using the mouse to click on that dialog puts us in a weird state where the drag ends at that mouseup. Not clicking on the modal (e.g. using the keyboard to dismiss it) is weirder still as a new drag session is apparently created after some timeout.

:enn suggests "Probably EventStateManager::mGestureDownContent is set while the modal dialog occurs. If that's the case, it should be cleared by calling StopTrackingDragGesture()",

There's more detail in https://bugzilla.mozilla.org/show_bug.cgi?id=1538460#c21

Comment 1

[:Gijs (he/him)]

Olli, I've been looking at this, bug 1542353 and bug 1593586. AFAICT for a "normal" modal prompt (Services.prompt.confirmExBC(docShell.browsingContext, Ci.nsIPrompt.MODAL_TYPE_WINDOW, "Hello", "Hello", 0, "OK", "Cancel", "", null, {})), this already works today, so nothing here needs to happen. Adding a call to StopTrackingDragGesture doesn't seem to matter.

However... the disruption to the drag operation is not limited to modal operations; the desktop notification (on Windows, at least, see testcase in bug 1593586 comment 4) and the download prompt (non-modal dialog, on nightly you may need to set browser.download.improvements_to_download_panel to false to repro) both run into this issue.

I can fix the downloads prompt (bug 1542353) by cheating and making the JS call opening the dialog call enterModalState and leaveModalState on the parent window, but that seems very wrong.

Do you have any ideas about where we should be stopping drags if not inside EnterModalState?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Ok, so sounds like the issue isn't about modal dialogs.
The (old?) download dialog is some kind of always-on-top-but-not-modal. So I'd look at either WindowWatcher code or Widget level code to clear current dnd when we open such thing.

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1544167 - end drag sessions when creating dialog chrome windows, r?smaug

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c738cce970b5
end drag sessions when creating dialog chrome windows, r=smaug

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c738cce970b5

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for causing perma failures on browser_popup_condition.js

• backout: https://hg.mozilla.org/integration/autoland/rev/2cd5daefdc1db96eb8ccce481a65e3f387022a87

• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=bFixtVE3RXmsBk_4868OoQ.0&revision=c738cce970b54a0ceaf75207945f8fd0db362c3b

• failure log: https://treeherder.mozilla.org/logviewer?job_id=361834668&repo=autoland&lineNumber=6600

• recent fail range: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=d14261ebd3b104c8712c296ddb52b2091096a6c9&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Copt%2Cmochitests%2Ctest-linux1804-64-qr%2Fopt-mochitest-browser-chrome-e10s%2Cbc3&tochange=7bd01907891724269de16463c0e99ed913d54ca1

• r/b range: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=bFixtVE3RXmsBk_4868OoQ.0&searchStr=Linux%2C18.04%2Cx64%2CWebRender%2Copt%2CMochitests%2Ctest-linux1804-64-qr%2Fopt-mochitest-browser-chrome-e10s%2Cbc3&tochange=c738cce970b54a0ceaf75207945f8fd0db362c3b&fromchange=6d8adc06d5d12376e9989f21e24c561cada866de

[task 2021-12-20T02:07:31.308Z] 02:07:31     INFO - TEST-PASS | toolkit/components/windowwatcher/test/browser_popup_condition.js | CHROME_SCROLLBARS should be present for features "noreferrer" - true == true - 
[task 2021-12-20T02:07:31.308Z] 02:07:31     INFO - TEST-PASS | toolkit/components/windowwatcher/test/browser_popup_condition.js | CHROME_TITLEBAR should be present for features "noreferrer" - true == true - 
[task 2021-12-20T02:07:31.309Z] 02:07:31     INFO - TEST-PASS | toolkit/components/windowwatcher/test/browser_popup_condition.js | CHROME_MENUBAR should be present for features "noreferrer" - true == true - 
[task 2021-12-20T02:07:31.309Z] 02:07:31     INFO - TEST-PASS | toolkit/components/windowwatcher/test/browser_popup_condition.js | CHROME_TOOLBAR should be present for features "noreferrer" - true == true - 
[task 2021-12-20T02:07:31.310Z] 02:07:31     INFO - TEST-PASS | toolkit/components/windowwatcher/test/browser_popup_condition.js | CHROME_PERSONAL_TOOLBAR should be present for features "noreferrer" - true == true - 
[task 2021-12-20T02:07:31.310Z] 02:07:31     INFO - Leaving test bound test_popup_conditions
[task 2021-12-20T02:07:31.310Z] 02:07:31     INFO - Buffered messages finished
[task 2021-12-20T02:07:31.311Z] 02:07:31     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/windowwatcher/test/browser_popup_condition.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. - 
[task 2021-12-20T02:07:31.311Z] 02:07:31     INFO - GECKO(7882) | MEMORY STAT | vsize 11618MB | residentFast 576MB | heapAllocated 274MB
[task 2021-12-20T02:07:31.312Z] 02:07:31     INFO - TEST-OK | toolkit/components/windowwatcher/test/browser_popup_condition.js | took 256381ms
[task 2021-12-20T02:07:31.313Z] 02:07:31     INFO - checking window state
[task 2021-12-20T02:07:31.313Z] 02:07:31     INFO - GECKO(7882) | Completed ShutdownLeaks collections in process 7882

Comment 7

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/2cd5daefdc1d

Comment 8

[:Gijs (he/him)]

Per conversations in #sheriffs, given this was on central for nearly 2 weeks, and given that this is only failing for Linux with hardware webrender enabled, whereas by default, the test only runs with software webrender - I am going to reland the patch as-is and file a follow-up for this test (which apparently is slower under linux hw webrender, for reasons we don't really know - there was an existing TV failure in bug 1739753, where the test actually fails on its very first run (ie somehow the preceding tests in the same folder are making it faster when run in its directory... so it's already pretty flaky).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22d928422120
end drag sessions when creating dialog chrome windows, r=smaug

Comment 10

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/22d928422120