711624 - JS prompt dialog will appear in the foreground when loading the page with the code in the background

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. go to http://people.mozilla.com/~nhirata/html_tp
2. long tap on PasswordPrompt.html
3. select Open Link in New Tab

Expected: dialog will appear after you switch to the tab
Actual: dialog appears in the current tab.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Since our JS prompt dialogs are not tab modal, we'll need to bring the page that spawned the dialog to the front.

Comment 2

[:Margaret Leibovic]

Attachment: patch

I basically just copied what desktop does:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/prompts/src/nsPrompter.js#415
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#2653

I also added a small optimization to selectTab to avoid sending unnecessary messages.

Comment 3

[:Margaret Leibovic]

Also, we should file a follow-up to get rid of the dead code in PromptService.js!

Comment 4

[Wesley Johnston (:wesj)]

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

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

Looks good!

::: mobile/android/chrome/content/browser.js
@@ +1889,5 @@
> +
> +        // We're about to open a modal dialog, make sure the opening
> +        // tab is brought to the front.
> +        let tab = BrowserApp.getTabForWindow(aEvent.target.top);
> +        BrowserApp.selectTab(tab);

I think because you're registered on the browser element, we should never receive this event when we can't get a tab here? I'm a bit worried about it, but I'm willing to give this a go and add in the check later if we determine we need it.

::: mobile/android/components/PromptService.js
@@ +772,5 @@
>      let data = Cc["@mozilla.org/android/bridge;1"].getService(Ci.nsIAndroidBridge).handleGeckoMessage(JSON.stringify({ gecko: aMsg }));
>      return JSON.parse(data);
> +  },
> +
> +  fireDialogEvent: function(domWin, eventName) {

aDOMWin and aEventName

Comment 5

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbd51fd8c2f7

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/bbd51fd8c2f7

Comment 7

[Cristian Nicolae (:xti)]

Everything works as expected on the latest Nightly. Closing bug as Verified Fixed

--
Firefox 15.0a1 (2012-05-21)
Device: Galaxy Nexus
OS: Android 4.0.2