734714 - Some prompts broken: "aDomWin is null" in prompt service

Status: VERIFIED FIXED

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

Description

[Zac Luzader]

Attachment: alogcat showing promptservice exceptions

The "add search engine" prompt (the one triggered by window.external.AddSearchProvider) and a dialog created by an addon (mbrubeck's "phony" -- user agent switcher) don't work. No prompt is displayed. alogcat shows the exception:

aDomWin is null on line 777 of /components/PromptService.js

Log snippets attached.
Log taken on an Ice Cream Sandwich tablet, but I can also duplicate it on an Android 2.3 Motorola Atrix, both running 2010-03-10 Nightly.

Comment 1

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

Regression from bug 711624. We need to check for a null domWin like desktop does here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/prompts/src/nsPrompter.js#493

Doing a null check for domWin here should do it:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/PromptService.js#159

Comment 2

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

Attachment: patch

simple one-liner. testing with phony as soon as the build is finished.

Comment 3

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

patch worked fine with phony

Comment 4

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

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

Margaret is on PTO

Comment 5

[Wesley Johnston (:wesj)]

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

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

This is what we did in XUL Fennec, and I think its fine. Desktop actually tries much harder to get a window. See:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/prompts/src/nsPrompter.js#493

which winds up calling WindowWatcher which does... a whole lot to ensure they find something. I... don't think its worth that much effort right now.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/261d255eb06d

Comment 7

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/261d255eb06d

Comment 9

[Cristian Nicolae (:xti)]

Verified fixed on:

Firefox 15.0a1 (2012-04-30)
Device: Samsung Captivate
OS: Android 2.2