Remove code for showModalDialog

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({dev-doc-complete})

Trunk
mozilla57
dev-doc-complete
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(3 attachments)

Bug 981796 changed the preferences to remove support for window.showModalDialog for non-e10s users as well as e10s users. For the moment we're leaving the code in so that stragglers who still use showModalDialog get one last chance before it's gone.

The current plan of record is to default showModalDialog to off in Firefox 56 and remove the code in 57. I'm taking this bug. Once we branch for 57, I'll whip up the patch to remove the code and the tests for good.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8894579 [details]
Bug 1374460 - Remove mochitests using showModalDialog.

https://reviewboard.mozilla.org/r/165078/#review170864

(MozReview has so nice dataloss issues when patch is removing files. Need to download the patch to see what actually gets removed)
Attachment #8894579 - Flags: review?(bugs) → review+

Comment 5

2 years ago
mozreview-review
Comment on attachment 8894580 [details]
Bug 1374460 - Remove all code related to showModalDialog.

https://reviewboard.mozilla.org/r/165080/#review170870
Attachment #8894580 - Flags: review?(bugs) → review+

Comment 6

2 years ago
mozreview-review
Comment on attachment 8894581 [details]
Bug 1374460 - Remove internal code that used to be used for showModalDialog.

https://reviewboard.mozilla.org/r/165082/#review170874

::: toolkit/components/browser/nsIWebBrowserChrome.idl:81
(Diff revision 1)
>  
> -    // Whether this was opened by nsGlobalWindow::ShowModalDialog.
> -    const unsigned long CHROME_MODAL_CONTENT_WINDOW   = 0x00080000;
> -
>      // Whether this window should use remote (out-of-process) tabs.
> -    const unsigned long CHROME_REMOTE_WINDOW          = 0x00100000;
> +    const unsigned long CHROME_REMOTE_WINDOW          = 0x00080000;

Would it be safer to keep using the old value? At least for now while some addons might still use this stuff.
Attachment #8894581 - Flags: review?(bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/487265c22f06
Remove mochitests using showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/0cc8d1037a36
Remove all code related to showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/193457157794
Remove internal code that used to be used for showModalDialog. r=smaug

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/487265c22f06
https://hg.mozilla.org/mozilla-central/rev/0cc8d1037a36
https://hg.mozilla.org/mozilla-central/rev/193457157794
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Keywords: dev-doc-needed
I documented this when the feature was disabled in 56:

https://developer.mozilla.org/en-US/Firefox/Releases/56
https://developer.mozilla.org/en-US/docs/Web/API/Window/showModalDialog

So I think we can ddc this. Let me know if you think anything else is needed. Thanks!
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.