759820 - Get rid of prompt service fallback in LoginManagerPrompter.js

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

We fall back to using the prompt service if we don't have a NativeWindow object, but I can't think of a case where that's necessary. Since this is a lot of extra code, I think we should get rid of it.

Comment 1

[Mark Capella [:capella]]

Attachment: Patch (v1)

Comment 2

[Mark Capella [:capella]]

Comment on attachment 654974 [details] [diff] [review]
Patch (v1)

I'm still learning my way around mobile/android ... but I think this is something close to what's being requested here ... let me know?

--mark

Comment 3

[:Margaret Leibovic]

Comment on attachment 654974 [details] [diff] [review]
Patch (v1)

This looks like it's on the right path. I've been trying to think hard about if our attempt to get NativeWindow will ever fail, and I don't *think* it will, but maybe we should still keep the error checks in place in _getNativeWindow, but upgrade the this.log(...) statements to Cu.reportError(...) so that we'll see them in logcat files if we start getting bug reports of problems here.

Also, I noticed that aNativeWindow is passed around a lot, but it's only ever actaul used in one place:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/LoginManagerPrompter.js#166

Now that we're not going to choose an alternate code path if this._getNativeWindow() returns null, we could just call this._getNativeWindow() in that one spot right before we call nativeWindow.doorhanger.show(...) and remove aNativeWindow as a parameter from that chain of function calls.

Comment 4

[Mark Capella [:capella]]

Attachment: Patch (v2)

Double checking my DIFF before asking for review ...

Comment 5

[Mark Capella [:capella]]

Comment on attachment 657020 [details] [diff] [review]
Patch (v2)

Looks good, unit testing on my device worked properly ...

Comment 6

[Mark Capella [:capella]]

Attachment: Patch (v3)

Fixed a nit...

Comment 7

[:Margaret Leibovic]

Comment on attachment 657230 [details] [diff] [review]
Patch (v3)

Nice cleanup.

Comment 8

[Mark Capella [:capella]]

push to TRY:
https://tbpl.mozilla.org/?tree=Try&rev=00c8a878e5c5

Comment 9

[Mark Capella [:capella]]

On to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=3e83ba39ec95

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/662cbbe93d65

Comment 11

[Cristian Nicolae (:xti)]

Changes were applied on the latest Nightly: http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/LoginManagerPrompter.js

Closing bug as verified fixed.