789721 - Check dom.mozBrowserFramesEnabled before BrowserElementPromptService initialized

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Comment 1

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch

To prevent from installing wrapped BrowserElementPromptService in application that don't use mozbrowser.

Pushed to try https://tbpl.mozilla.org/?tree=Try&rev=22cd4454a899

Comment 2

[Justin Lebar (not reading bugmail)]

Mark, can you verify that this fixes the problem for you?  If the commit mentioned in bug 789572 is actually the problem, I'd be surprised if this fixed anything.

Comment 3

[Justin Lebar (not reading bugmail)]

Comment on attachment 659516 [details] [diff] [review]
Patch

Please add code to ensure that we don't run _init() twice.

r=me with that change, but I'd like someone to verify that this actually fixes the problem.

Comment 4

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch

Comment 5

[Justin Lebar (not reading bugmail)]

Comment on attachment 659546 [details] [diff] [review]
Patch

Maybe this is a nit, but I'd move the |this._initialized = true| line earlier in _init() because if we throw  an exception in the middle of _init(), I think we should just give up, rather than try to re-initialize when the pref changes again.

r+ means you don't need to re-request review on the next patch, but I would prefer to hear from mfinkle if this solves their problem before checking it in.

Comment 6

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

I tested the Try build and it seems to work fine. I get the prompt when trying to install a webapp and when trying to install an add-on

Comment 7

[Justin Lebar (not reading bugmail)]

Since it's a trivial change and we want to get it in the next nightly, I've pushed Patrick's patch with my last nit addressed:

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

Comment 8

[(PTO May 7-12) Ryan VanderMeulen [:RyanVM]]

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