Remove code for showModalDialog

RESOLVED FIXED in Firefox 57

Status

()

Core
DOM
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({dev-doc-complete})

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

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

8 months ago
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

7 months 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

7 months 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

7 months 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

7 months 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

7 months 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: 7 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Assignee)

Updated

7 months ago
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.