735237 - window.showModalDialog causes weird issues

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See url testcase, steps to reproduce:
- Tap on the window showModalDialog() button (second to last button)

Expected result:
- A new window opens with that page, which has an input a 'Close' button, a link and some text in it.

Actual result:
- The page seems to get opened in the same window, it's completely zoomed in, so that nothing is visible. Tapping on the Android back button doesn't seem to go back to the previous page.

Comment 1

[Matt Brubeck (:mbrubeck)]

Note: XUL Fennec throws an exception (NS_ERROR_FAILURE in nsIDOMWindow.showModalDialog) and fails to open a window.

It's not clear how we could/should support modal content windows in Fennec.  Proper support would require that we freeze content in the main window and prevent the user from interacting with it until the modal page is dismissed.  Unless we have a good UX design, it's probably best to throw an exception like XUL Fennec.

For comparison, none of the main competing browsers handle showModalDialog correctly:

* Opera Mobile does not open the URL, but displays a dialog with just an "OK" button.
* Stock Android Browser does not open the URL, but displays a dialog with the text "undefined" (in both Android 2.3 and 4.0)
* Chrome opens the URL in a new tab but the tab is not modal (on both Android and desktop).

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This patch makes window.showModalDialog throw an exception in fennec.  Johnny, do you think this is the best approach and implementation?  I'd also like to do basically the same thing for window.print in bug 739038.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Oops, accidentally uploaded an empty file.  (Thanks, Margaret.)

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 614202 [details] [diff] [review]
patch

This makes sense to me until we have the UX bandwidth to figure out exactly how this should work on a mobile device, if at all.

Comment 5

[Mozilla RelEng Bot]

Try run for e9b15bc58060 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e9b15bc58060
Results (out of 55 total builds):
    success: 48
    warnings: 6
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e9b15bc58060

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: patch with test

Same as the previous patch but adds a simple mochitest.

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e7aa45f40f7

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/3e7aa45f40f7

Comment 9

[Cristian Nicolae (:xti)]

An error occurs when showModalDialog() button is tapped:

E/GeckoConsole(25018): [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMWindow.showModalDialog]" {file: "http://people.mozilla.org/~mwargers/tests/dialogs/winopendialog.htm" line: 14}]

Since this is expected (comment #2), this bug will be closed as verified fixed on:

Firefox 15.0a1 (2012-05-30)
Firefox 14.0a2 (2012-05-30)

Device: Galaxy Nexus
OS: Android 4.0.2