Closed Bug 1077002 Opened 5 years ago Closed 5 years ago

window.showmodaldialog does not work with e10s

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
e10s m5+ ---
firefox39 --- fixed

People

(Reporter: offbynone, Assigned: mrbkap)

References

()

Details

(Keywords: productwanted)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141002030202

Steps to reproduce:

trigger window.showmodaldialog with e10s enabled


Actual results:

nothing


Expected results:

modal dialog should have opened
See also bug 981796
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 981796
I don't think it's appropriate to mark this as a DUPE given that this bug and that bug asks for two completely opposite things.

Is the plan to not add support for showModalDialog for e10s? That would certainly be ok with me, but might affect the timing of when we need to remove support for showModalDialog (previously planned for Firefox 39).

Is there a schedule for when we're planning on turning on e10s?
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
See Also: → 981796
Blake is going to follow up with Jonas.
Assignee: nobody → mrbkap
tracking-e10s: --- → ?
Component: Untriaged → DOM
OS: Windows 7 → All
Product: Firefox → Core
Hardware: x86_64 → All
Version: 35 Branch → Trunk
Attached patch patch v1Splinter Review
We've decided to remove showModalDialog before e10s is enabled by default, but there's going to be a small lag before we can do that.  Until then, the error that we throw in e10s is pretty cryptic (NS_ERROR_UNEXPECTED) and we assert in debug builds. This patch gets rid of the assertion as well as giving a slightly better message (NS_ERROR_NOT_AVAILABLE) which, combined with the WarnOnceAbout should lead developers pretty quickly to the source of the problem.
Attachment #8569570 - Flags: review?(bobbyholley)
Comment on attachment 8569570 [details] [diff] [review]
patch v1

Review of attachment 8569570 [details] [diff] [review]:
-----------------------------------------------------------------

Can we add a test for this? r=me with that
Attachment #8569570 - Flags: review?(bobbyholley) → review+
Attached patch Add a test.Splinter Review
Attachment #8570164 - Flags: review?(jmathies)
Comment on attachment 8570164 [details] [diff] [review]
Add a test.

Review of attachment 8570164 [details] [diff] [review]:
-----------------------------------------------------------------

looks good.
Attachment #8570164 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/mozilla-central/rev/3065c0007569
https://hg.mozilla.org/mozilla-central/rev/a4a1d5217e5b
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
I came across this issue today within Office 365 and have finally tracked it down to e10s. I'm using Firefox 40.0a2 (2015-06-09) and window.showModalDialog() doesn't seem to be working, at least, within Office 365?
This still ahsn't been fixed yet you have now made this behaviour default? Breaking websites for lots of people.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> https://hg.mozilla.org/mozilla-central/rev/3065c0007569
> https://hg.mozilla.org/mozilla-central/rev/a4a1d5217e5b

This still isn't fixed 4 months on from my original comment?

Name 	Firefox
Version 	43.0a2
Build ID 	20151006004017
Flags: needinfo?(mrbkap)
This is still broken.  Basically, e10s is useless to me for this reason (I manage Office 365 all day long).  Not to mention that I need LastPass working...but that's another story.  This should not be marked resolved/fixed, it is not.

Firefox Beta 44.0b1
We do not plan on fixing this, we want to deprecate support for it. See comment 5.
Flags: needinfo?(mrbkap)
Keywords: productwanted
Hrrrmmmff.  I missed that comment.  I guess someone needs to inform the devs at Microsoft.  This also breaks Exchange 2013 and Exchange 2016 management as well (not just Office 365).  This is deal breaker all around for me and for all of my co-workers.  Even if LastPass ever becomes compatible...
Hmm, so chromium deprecated this back in 2014, but they offered enterprises a way to turn it back on [1] temporarily. Looks like they turned it completely off [2][3] in Chrome 43. I think maybe it's time to do the same, there are various ways of working around it. I've marked this for productwanted for review, just in case.

[1] http://blog.chromium.org/2014/07/disabling-showmodaldialog.html
[2] https://developer.mozilla.org/en-US/docs/Web/API/Window/showModalDialog
[3] https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/Xp1qU_SeLEk/cJQRy7oOKXYJ
Depends on: 981796
Thanks Jim.  I did a bit of research on my own afterwards and came across the same articles.  Strangely, it appears that at least for Office 365 and Exchange 2016, the showModalDialog is avoided and not even used in Edge and Chrome and mailbox management works fine; the problem areas just open in a new tab and avoid the use of the showModalDialog all together.  It's not just the user agent that changes the behavior either as I tried using Chrome 47 UA in Firefox.  So it's not they these examples don't support the browser not having showModalDialog, it's that they still believe the browser can handle it.
Does it fix the problem to set dom.disable_window_showModalDialog to true via about:config and restart Firefox?
Note that the value does not exist by default, so you will need to create the value.
Flags: needinfo?(jason)
That fixed it.  I recommend we automatically override that and set to true if browser.tabs.remote.autostart is true from now on so others don't run into this.  Now if only LastPass worked correctly...
Flags: needinfo?(jason)
See Also: → 1234700
(In reply to Jason from comment #19)
> That fixed it.  I recommend we automatically override that and set to true
> if browser.tabs.remote.autostart is true from now on so others don't run
> into this.

Exactly. Filed bug 1234700.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.