Closed
Bug 981796
Opened 11 years ago
Closed 7 years ago
Remove window.showModalDialog
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla56
People
(Reporter: eric, Assigned: mrbkap)
References
Details
(4 keywords, Whiteboard: See comment 25, 26 [platform-rel-Microsoft][platform-rel-Outlook][webcompat])
Attachments
(5 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
nalexander
:
review+
|
Details |
Chrome plans to remove window.showModalDialog from Chrome M35:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/xh9fPX0ijqk/ixHZCOH6GLgJ
It looks like you all plan to remove it:
https://developer.mozilla.org/en-US/docs/Web/API/Window.showModalDialog
but I didn't see a bug on file.
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Yes. Our plan is definitely to remove this. We've been sending out deprecation warnings since Firefox 28.
We *might* need to get telemetry data from Gecko in order to catch browser-specific code paths, but I'm not sure if we'll wait for that or not.
Keywords: addon-compat,
dev-doc-needed
Comment 2•11 years ago
|
||
I don't think we should be removing this until Chrome has shipped their removal and we've had a chance to see what the response is, at the very least.
Keywords: compat
I'm ok with waiting for that yeah (we've been doing our fair share of going first with potentially-webcompat-breaking-changes lately).
However we could also just land an implementation that enables preffing this on/off and flipping that pref to off. That way we can easily pref back on even on later branches if it turns out Google runs into too much trouble.
Comment 4•11 years ago
|
||
The "dom.disable_window_showModalDialog" preference exists. It doesn't hide the API (that would depend on bug 789261), so it would fail object-detection stuff for people who want to use showModalDialog if available but fall back to something else if not, but it does make the API throw when called. So flipping that pref is strictly less web-compatible than removing the API altogether, I guess. ;)
Yeah, I think we need the pref to hide the API as well. According to the thread in comment 0 there are pages that feature detect the API (and I seem to recall that being the case when we first implemented the API too).
Reporter | ||
Comment 6•11 years ago
|
||
crbug.com/345831 is the Chrome bug on the subject.
Comment 7•11 years ago
|
||
Should we write a hacks blog post about the upcoming removal or something? Blink got into some trouble because of lack of enough communication on this.
Comment 8•11 years ago
|
||
We should, imo, start with telemetry, just to avoid crying wolf.
Comment 9•11 years ago
|
||
Yes, some telemetry, hopefully for FF31 (which will be an ESR too), and perhaps remove
in FF32.
If we get WebIDL-fied Window for FF32, disabling SMD would be rather simple.
Comment 10•11 years ago
|
||
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #7)
> Should we write a hacks blog post about the upcoming removal or something?
> Blink got into some trouble because of lack of enough communication on this.
FWIW, we (at Opera) just published such a blog post: http://dev.opera.com/articles/view/showmodaldialog/ It refers to this bug ticket. Let me know if any Firefox-specific info is missing and I’ll happily add it.
Updated•11 years ago
|
Comment 11•11 years ago
|
||
(In reply to comment #10)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #7)
> > Should we write a hacks blog post about the upcoming removal or something?
> > Blink got into some trouble because of lack of enough communication on this.
>
> FWIW, we (at Opera) just published such a blog post:
> http://dev.opera.com/articles/view/showmodaldialog/ It refers to this bug
> ticket. Let me know if any Firefox-specific info is missing and Iâll happily
> add it.
That's a great blog post! Thanks for making it.
Updated•11 years ago
|
Keywords: site-compat
Comment 12•10 years ago
|
||
I'm so happy to see this dialog going away!
If it ends up being difficult to just dump it... Another possibility I had discussed with someone a long time ago was to put it behind a permission prompt / popup blocker. If a site wants to use it, the user would have to allow/whitelist it. (This was motivated by showModalDialog having potential for being user-hostile, but supposedly being necessary on unknown intranet sites.)
Comment 13•10 years ago
|
||
Hi there,
we're developers of a huge business app and we absolutely need modal dialogs.
With Chrome 37 we got in big trouble, because we had to tell the users to switch browsers.
Please give us at least a user option, so our users can work with modal dialogs.
Marco
Comment 14•10 years ago
|
||
I don't get how you could not easily replace any modal dialog by a pop-in + iframe.
IMHO it's far from a web-breaker update…
Comment hidden (off-topic) |
Comment 16•10 years ago
|
||
Marco, Chrome has a Group Policy setting to enable showModalDialog for ease of the transition.
http://www.chromium.org/administrators/policy-list-3#EnableDeprecatedWebPlatformFeatures
Note that it will be removed eventually.
Comment 17•10 years ago
|
||
Thanks Kimura, we read it, but we are to stupid to get it to work.
And we're suppose our customers (non IT folks, just SaaS-Users) are too.
Comment 18•10 years ago
|
||
Marco, this setting does work only for PC in domains.
This reg-file works in domain for me:
Windows Registry Editor Version 5.00
[HKEY_CURRENT_USER\Software\Policies\Google\Chrome\EnableDeprecatedWebPlatformFeatures]
"1"="ShowModalDialog_EffectiveUntil20150430"
Comment 19•10 years ago
|
||
Hi Alexey,
thanks a lot. Our windows customers are no problem - they just switch to IE (most of them are not in domains, just simple Cloud-solution-users). More a problem are the mac users. But Safari 7 is quite good, so we told them to switch to. (its a lot easier to tell them to switch browser then to tell them to install the business admistration toolkit required for the chrome option)
I just hope, with Firefox doesn't happen the same. So I'm begging to leave this function in or if it really should be removed to provide an option for the user to activate it again.
Comment 20•10 years ago
|
||
We don't have particular hurry to remove showModalDialog from Firefox as comment 2 hints.
In Blink showModalDialog does cause certain technical issues which we don't have in Gecko.
(I wonder if Webkit has the same issues as what Blink has.)
Comment 21•10 years ago
|
||
Hi,
we have the same problem as Marko. Modal dialogs are very important in business apps to interact with users. In some cases synchronous processing is needed and we cannot replace it with asynchronous one. The other thing is that replacement of showModalDialog function needs a lot of time.
MJ
Ps. What is wrong with showModaldialog on Android - it doesn't work!
Comment 22•10 years ago
|
||
There's now the <dialog> element which can be used for modal dialogs. There's also a polyfill for browsers that do not implement the element yet. Check out demo.agektmr.com/dialog/ for further info and the WHATWG HTML5 spec for … the spec!
There is no reason showModalDialog() is needed at all. Even if you don't use the <dialog> element, the functionality can be achieved with a simple JS library.
Comment 23•10 years ago
|
||
Florian, we tried every suggestion so far. But there is no replacement for modal dialogs. Neither the <dialog>-element nor any js library.
It's just that simple. I need a function
sVal = gsGetValueFromUser()
which opens a html page of my choice and gets back a result.
No callbacks, just a function with a result.
And without showModalDialog() there is no way, you can do it.
No callbacks, because we use such functions in loops, in libraries and in nested calls. And callbacks would destroy the clean structure of this code. (which in our case runs for tens of thousends of users for 15 years)
Comment 24•10 years ago
|
||
First and the most important thing of all is that ShowModalDIalog() makes that function is waiting for result from modal window.
Simple sites can be changed in easy way but business app can not - there is too many realtions between different part of the system which process one-by-one or in loops.
To be clear:
* Yes, we are aware that there is no simple replacement which enables you to write code like
var val = getValueFromUser();
* However, it is possible to get *exactly* the same UX as showModalDialog() by using features like
<dialog> (and polyfills for it) and window.open.
* We are aware that this will require some code changes on your side in order to make the code be
asynchronous. In many cases these code changes will be non-trivial.
So, put it another way. This will require non-trivial code changes as there's no simply drop-in replacement. However you can still get the exact same UX for your users.
Because of the need for non-trivial code changes, we have announced this change so far in advance. This is also why we quite a while ago added a warning that this function was deprecated. To give you guys time to make the necessary code changes to maintain the current UX without using showModalDialog().
But make no mistake. We *will* remove this function. It's just a question of when. So I'd recommend starting on the necessary code changes.
The reason we are getting rid of showModalDialog() is that it results in bad UI for users since the rest of the process is locked up. And it results in a lot of code complexity for browsers, which is likely why we haven't gotten it working on Android.
This complexity is getting worse as we are making other changes to the browser, like e10s.
Whiteboard: See comment 25
Regarding timing, I would say that we can commit to not removing this function before Firefox 39. This means that the function will be around until around mid June 2015.
It also means that for enterprise you can switch to using ESR 38 builds, which will have the function into mid 2016.
https://www.mozilla.org/en-US/firefox/organizations/faq/
But again, the function *will* go away.
Whiteboard: See comment 25 → See comment 25, 26
Comment 27•10 years ago
|
||
I've made it clear in the doc (with our timing, but also with Chrome timing that was already in the article before) that this method is going away. Hope it will help a bit in spreading the word.
https://developer.mozilla.org/en-US/docs/Web/API/Window.showModalDialog
Comment 28•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #26)
> Regarding timing, I would say that we can commit to not removing this
> function before Firefox 39. This means that the function will be around
> until around mid June 2015.
Ok, so there is time to make changes.
> The reason we are getting rid of showModalDialog() is that it results in bad UI for users since
> the rest of the process is locked up. And it results in a lot of code complexity for browsers,
> which is likely why we haven't gotten it working on Android.
How about firefox mobile ? (Andorid). showModalDialog() will be working till version 39 too?
In ff32 (mobile) showModalDialog() is not working.
Is this situation temporary (is is a bug and you'll fix it) or is it a permanent (and sudden) solution?
Our app worked on tablets with android (ff 31) , now it doesn't (ff32). What should I tell to users?
I don't know why showModalDialog does not work on android, or if that was an intentional change or an accidental regression.
I would suggest filing a separate bug specifically to investigate that. Feel free to cc me.
Comment 30•10 years ago
|
||
Bug 753555 is about implementing showModalDialog on Android.
See Also: → 753555
Comment 31•10 years ago
|
||
We shouldn't drop the support without an replacement.
Depends on: dialog-element
Comment 32•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #26)
> Regarding timing, I would say that we can commit to not removing this
> function before Firefox 39. This means that the function will be around
> until around mid June 2015.
How about disabling it, say, starting now for pre-release builds? (Nightly/Aurora, maybe Beta later) That will start to give us some early feedback on the impact of removal.
Updated•10 years ago
|
Assignee: nobody → mrbkap
tracking-e10s:
--- → m4+
See Also: → 1077002
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 34•10 years ago
|
||
I filed bug 1137025 to get telemetry on how often showModalDialog is used in the wild. I don't suspect any real suprises there. Once we have the results, we can start with this patch to turn off showModalDialog (and then later we'll be able to rip the code out).
Note that at this time, we do not intend to implement showModalDialog for e10s. When we switch to e10s on be default would probably be a good point to rip the code out entirely.
Comment 35•10 years ago
|
||
Will telemetry count internal network usage correctly?
Comment 36•10 years ago
|
||
FYI, the changes to fully remove showModalDialog just landed in Chrome (http://crbug.com/345831) and will take effect in M43. Good luck to you guys.
Comment 37•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #26)
> Regarding timing, I would say that we can commit to not removing this
> function before Firefox 39. This means that the function will be around
> until around mid June 2015.
Friendly reminder (Firefox Developer Edition is now at 39). :-)
Project Spartan has removed showModalDialog: https://twitter.com/jacobrossi/status/588353411168346113
Comment 38•10 years ago
|
||
The difference with Chrome is they support HTML5 <dialog> elements. Removing this feature seems to introduce a gap in feature set. Does FFX team expect to prioritize https://bugzilla.mozilla.org/show_bug.cgi?id=840640
Comment 39•9 years ago
|
||
I use a local website "windows.showModalDialog" I wonder what the real deadline for it to stop working?
Thank you.
Comment 40•9 years ago
|
||
How are we doing here?
Comment 41•9 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/window-showmodaldialog-will-be-removed/
Firefox 45 will be the next ESR so the target milestone could be 46 maybe?
Comment 42•9 years ago
|
||
Would you guys please implement HTML5 <Dialog> before you remove showModalDialog?
https://bugzilla.mozilla.org/show_bug.cgi?id=840640
Comment 43•9 years ago
|
||
Recently showModalDialog() has been removed from the spec.
https://github.com/whatwg/html/commit/eec96462b5bd1c7f904a4880bc04dade6efcd597
Comment 44•9 years ago
|
||
Removing this is going to break attachments in Outlook Web App 2010 and reportedly Exchange webmail (see https://github.com/whatwg/html/pull/374#issuecomment-171466693). Chrome and Edge are OK with that but if we go the same route I'd like to avoid us thinking doing evangelism is going to fix those problems.
Comment 45•9 years ago
|
||
Can we make an extension to fix old versions of OWA? Either monkeypatch their code, or shim showModalDialog on top of another semi-synchronous feature such as XHR.
Comment 46•9 years ago
|
||
We should consider removing showModalDialog BEFORE shipping e10s, to avoid giving users too many reasons to stay on old versions at the same time (and to avoid swamping evang teams).
Comment 47•8 years ago
|
||
Updated the site compat doc as Firefox 48 will enable e10s by default and showModalDialog() is practically gone.
https://www.fxsitecompat.com/en-CA/docs/2016/window-showmodaldialog-has-been-removed/
Comment 48•8 years ago
|
||
(In reply to Jesse Ruderman from comment #45)
> Can we make an extension to fix old versions of OWA? Either monkeypatch
> their code, or shim showModalDialog on top of another semi-synchronous
> feature such as XHR.
Best solution would be to patch the server. MS has fixes for both Exchange 2010 and Exchange 2013.
Comment 49•8 years ago
|
||
Mike, can we use the system addon to monkey patch these old versions of OWA?
Flags: needinfo?(miket)
Comment 50•8 years ago
|
||
Yeah, I think so. That's one of my use cases in the explainer doc I wrote -- will send to dev-platform on Monday (to get more off-PTO eyeballs on it).
Flags: needinfo?(miket)
Updated•8 years ago
|
platform-rel: --- → ?
Whiteboard: See comment 25, 26 → See comment 25, 26 [platform-rel-Microsoft][platform-rel-Outlook]
Comment 51•8 years ago
|
||
When do you want to implement the dialog tag html5?
Updated•8 years ago
|
Whiteboard: See comment 25, 26 [platform-rel-Microsoft][platform-rel-Outlook] → See comment 25, 26 [platform-rel-Microsoft][platform-rel-Outlook][webcompat]
Updated•8 years ago
|
platform-rel: ? → -
Comment 53•8 years ago
|
||
Blake, do we have some sort of official "we're not going to ship showModalDialog" statement? Are we going to ship the pref to turn it off in non-e10s builds, too? Should we close this as FIXED?
Flags: needinfo?(mrbkap)
Comment 54•8 years ago
|
||
According to the current situation, we disabled this feature on 51.0 in some builds. Is it about e10s? Maybe we should have an official statement.
Comment 55•8 years ago
|
||
My unofficial note is already here, but someone has to update the official MDN article as well.
https://www.fxsitecompat.com/en-CA/docs/2016/window-showmodaldialog-has-been-removed/
Assignee | ||
Updated•7 years ago
|
Attachment #8569568 -
Attachment is obsolete: true
Flags: needinfo?(mrbkap)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 57•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 60•7 years ago
|
||
mozreview-review |
Comment on attachment 8875516 [details]
Bug 981796 - Turn off window.showModalDialog.
https://reviewboard.mozilla.org/r/146942/#review152836
It would be nice to have support for <dialog> (not that <dialog> is very well designed replacement).
But ok, at least we should behave consistently.
Attachment #8875516 -
Flags: review?(bugs) → review+
Comment 61•7 years ago
|
||
mozreview-review |
Comment on attachment 8876923 [details]
Bug 981796 - Don't assume that the active global is our global.
https://reviewboard.mozilla.org/r/148216/#review152842
As long as we have the implementation for the feature in tree, we should have tests for it.
Attachment #8876923 -
Flags: review?(bugs) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 66•7 years ago
|
||
mozreview-review |
Comment on attachment 8878676 [details]
Bug 981796 - Remove WPTs that assume we implement showModalDialog.
https://reviewboard.mozilla.org/r/149980/#review154788
Attachment #8878676 -
Flags: review?(bugs) → review+
Comment 67•7 years ago
|
||
mozreview-review |
Comment on attachment 8878677 [details]
Bug 981796 - Make tests that use showModalDialog pass.
https://reviewboard.mozilla.org/r/149982/#review154790
rs+
Attachment #8878677 -
Flags: review?(bugs) → review+
Comment 68•7 years ago
|
||
mozreview-review |
Comment on attachment 8876923 [details]
Bug 981796 - Don't assume that the active global is our global.
https://reviewboard.mozilla.org/r/148216/#review154792
Do we need this on branches?
Attachment #8876923 -
Flags: review?(bugs) → review+
Comment 69•7 years ago
|
||
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f91ff196c93
Turn off window.showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/5a1e684a1494
Don't assume that the active global is our global. r=smaug
https://hg.mozilla.org/integration/autoland/rev/eca1651af3d3
Remove WPTs that assume we implement showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/fe6569eae4ef
Make tests that use showModalDialog pass. r=smaug
Assignee | ||
Comment 70•7 years ago
|
||
Assignee | ||
Comment 71•7 years ago
|
||
Comment on attachment 8876923 [details]
Bug 981796 - Don't assume that the active global is our global.
Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: Possible crashes with (mis)uses of showModalDialog.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's changing code to follow an assertion more closely instead of assuming current conditions.
[String changes made/needed]: None.
Attachment #8876923 -
Flags: approval-mozilla-esr52?
Attachment #8876923 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 72•7 years ago
|
||
I filed bug 1374460 on removing the code/tests entirely. I'll write the patch for it as soon as we branch for 57.
Backed this out for failures on android like https://treeherder.mozilla.org/logviewer.html#?job_id=108348619&repo=autoland
https://hg.mozilla.org/integration/autoland/rev/6ed0b32285d2d063884d98cba1aab3d3318f192d
Flags: needinfo?(mrbkap)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mrbkap)
Comment 75•7 years ago
|
||
mozreview-review |
Comment on attachment 8879348 [details]
Bug 981796 - Disable this test on Android.
https://reviewboard.mozilla.org/r/150634/#review156310
I think this is fine. On IRC, I reasoned as follows:
```
It's doubtful that Fennec itself uses these modal dialogs, right? Since we've already set disable to true.
So this test, if it sets it to true, almost certainly can't work on Fennec right now.
```
and mrbkap concurs.
Roll on!
Attachment #8879348 -
Flags: review+
Comment 76•7 years ago
|
||
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ab4610277fe
Turn off window.showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/abf75ca9eede
Don't assume that the active global is our global. r=smaug
https://hg.mozilla.org/integration/autoland/rev/bd0ffaac1d66
Remove WPTs that assume we implement showModalDialog. r=smaug
https://hg.mozilla.org/integration/autoland/rev/bae6691021e3
Make tests that use showModalDialog pass. r=smaug
https://hg.mozilla.org/integration/autoland/rev/cf345d031b95
Disable this test on Android. r=nalexander
Comment 77•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7ab4610277fe
https://hg.mozilla.org/mozilla-central/rev/abf75ca9eede
https://hg.mozilla.org/mozilla-central/rev/bd0ffaac1d66
https://hg.mozilla.org/mozilla-central/rev/bae6691021e3
https://hg.mozilla.org/mozilla-central/rev/cf345d031b95
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Updated•7 years ago
|
Attachment #8879348 -
Flags: review?(snorp)
Comment 78•7 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #71)
> Comment on attachment 8876923 [details]
> Bug 981796 - Don't assume that the active global is our global.
>
> Approval Request Comment
> [Feature/Bug causing the regression]: n/a
> [User impact if declined]: Possible crashes with (mis)uses of
> showModalDialog.
Can you expand on what's triggering this uplift? What crash volume are we talking about? Thanks.
Flags: needinfo?(mrbkap)
Assignee | ||
Comment 79•7 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #78)
> Can you expand on what's triggering this uplift? What crash volume are we
> talking about? Thanks.
After investigating further, we don't need to backport this patch. I was a little worried that this could be a security problem (compartment mismatch), but the assertion that I was seeing locally involved two parameters that are only used for the assertion. The rest of the code does the right thing.
Flags: needinfo?(mrbkap)
Assignee | ||
Updated•7 years ago
|
Attachment #8876923 -
Flags: approval-mozilla-esr52?
Attachment #8876923 -
Flags: approval-mozilla-beta?
Comment 80•7 years ago
|
||
I've documented this. The main Window page has been updated to put it here:
https://developer.mozilla.org/en-US/docs/Web/API/Window#Obsolete_methods
I've updated the notes on the showModalDialog() ref page:
https://developer.mozilla.org/en-US/docs/Web/API/Window/showModalDialog
And I've added a note to the Fx55 rel notes:
https://developer.mozilla.org/en-US/Firefox/Releases/55#APIs_2
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Updated•3 years ago
|
No longer depends on: dialog-element
See Also: → dialog-element
You need to log in
before you can comment on or make changes to this bug.
Description
•