Closed Bug 1374460 Opened 3 years ago Closed 3 years ago

Remove code for showModalDialog

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

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 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 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 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+
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
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!
Component: DOM → DOM: Core & HTML
Depends on: 1587922
You need to log in before you can comment on or make changes to this bug.