1704616 - Enable proton modals

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, enhancement, P1)

Description

[:Gijs (away/busy until Mar 10)]

Per discussion with Romain, we want to enable proton modals on nightly in their current state.

Comment 1

[:Gijs (away/busy until Mar 10)]

Welp, let's see how bad it is: https://treeherder.mozilla.org/jobs?repo=try&revision=cabf5e40b9404ba3c80570c6195ef0f11f9cba3b

Comment 2

[:Gijs (away/busy until Mar 10)]

Failing tests:

browser/base/content/test/tabdialogs/browser_tabdialogbox_content_prompts.js
browser/components/places/tests/browser/browser_addBookmarkForFrame.js

annnd crashes in bug 1699844. It's quite possible that the crashes hide further test failures. :-(

Comment 3

[:Gijs (away/busy until Mar 10)]

I have fixes for the two tests in comment #2 but there are more places tests that are failing once I fix the addBookmarkForFrame one. Those seem largely to do with the helper that opens the bookmarks properties dialog, with context menus staying open when code was relying on the new window opening to close the menu and remove focus from it (so stray keypresses don't go there) and code being confused about whether to expect the dialog to open in a subdialog in the browser window or not. I hope to have a patch ready in the next few hours.

Comment 4

[:Gijs (away/busy until Mar 10)]

Attachment: Bug 1704616 - fix modal tests that break with proton modals enabled, r?mak

Comment 5

[:Gijs (away/busy until Mar 10)]

Attachment: Bug 1704616 - work around promiseDocumentFlushed crashes in proton modal prompts, r?mconley

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/646a650576a9
fix modal tests that break with proton modals enabled, r=mak

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/646a650576a9

Comment 8

[:Gijs (away/busy until Mar 10)]

Attachment: Bug 1704616 - enable proton modals by default

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/623c1b4ecf35
work around promiseDocumentFlushed crashes in proton modal prompts, r=mconley
https://hg.mozilla.org/integration/autoland/rev/16e0111f7c6d
enable proton modals by default r=jaws

Comment 10

[:Gijs (away/busy until Mar 10)]

Attachment: Bug 1704616 - fix browser_beforeunload_urlbar.js to not crash in promiseDocumentFlushed, take #2

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be83f7c46c9c
fix browser_beforeunload_urlbar.js to not crash in promiseDocumentFlushed, take #2

Comment 12

[Dorel Luca [:dluca]]

Backed out changeset be83f7c46c9c (bug 1704616) for Mochitest failures in remote/cdp/test/browser/page/browser_javascriptDialog_prompt.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=336523785&repo=autoland&lineNumber=2429

https://treeherder.mozilla.org/logviewer?job_id=336531304&repo=autoland&lineNumber=19048

https://treeherder.mozilla.org/logviewer?job_id=336526531&repo=autoland&lineNumber=142963

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=6607000204c56c89613f1637f7df93b53689907c

Backout:
https://hg.mozilla.org/integration/autoland/rev/ba0f8adbb582486a20bfc7345317805cde2f8752

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b707acf2003
Backed out 2 changesets for Mochitest failures in remote/cdp/test/browser/page/browser_javascriptDialog_prompt.js. CLOSED TREE

Comment 14

[:Gijs (away/busy until Mar 10)]

Well, patches from 1699844 are landing now, so I pushed to try to see if that means we can flip the pref without having to work around that crash: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7593edb99ef18efc0600aaea5e92c54122594077

Comment 15

[:Gijs (away/busy until Mar 10)]

Attachment: Bug 1704616 - handle promise rejections from promiseDocumentFlushed in case the modal window goes away, r?mconley,jaws,mtigley

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43c14056afe4
handle promise rejections from promiseDocumentFlushed in case the modal window goes away, r=mconley
https://hg.mozilla.org/integration/autoland/rev/7a8ac810e5a1
enable proton modals by default r=jaws

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/43c14056afe4
https://hg.mozilla.org/mozilla-central/rev/7a8ac810e5a1