Closed Bug 1498233 Opened Last year Closed Last year

Password Doorhanger is not displayed using custom tabs


(Firefox for Android :: Custom Tabs, defect)

Firefox 64
Not set



Firefox 65
Tracking Status
firefox63 --- unaffected
firefox64 --- verified
firefox65 --- verified


(Reporter: sflorean, Assigned: mconley)



(Keywords: regression)


(1 file)

Device: Google Pixel(Android 9), Samsung Galaxy S8(Android 8.0);
Build: Nightly 64.0a1 (2018-10-10);

Steps to reproduce:
1. Open Twitter from Gmail using custom tabs;
3. Enter credentials and tap on "Log in".

Expected result:
A Password Doorhanger is displayed on the left side of the URL Bar asking to save the credentials.

Actual result:
The password doorhanger is not displayed.
Last good revision: f69ecb2abf86e239c528a27f394e88019bd7cdae
First bad revision: cfa50dc1f61600b08d966210d0319b052ac86627
NI mconley for regression.
Flags: needinfo?(mconley)
Hey jimchen, I seem to recall you working on login manager stuff for custom tabs... any idea how this revision might have broken the login prompt for custom tabs? I really don't know where to start to debug this.
Flags: needinfo?(mconley) → needinfo?(nchen)
Nothing from that patch jumps out. TBH another revision [1] that landed around the same time looks more relevant because that patch actually touched mobile code.

Flags: needinfo?(nchen)
Thanks, jchen.

Hey Stefan, can you please confirm the regression range?

cc'ing MattN just in case.
Flags: needinfo?(stefan.deiac)
Hey Mike, the regression range it's good.
Device: OnePlus 5T (Android 8.1.0).
Flags: needinfo?(stefan.deiac)
Hi Matt, can you take a look at this?
Flags: needinfo?(MattN+bmo)
I really would rather not as I warned during review that the change would likely break something outside desktop Firefox so Mike offered to deal with that before landing[1] and I won't be able to do Android development for a month anyways. Also, this is probably much easier for an Android developer to fix. If Password Manager was actually resourced/supported properly them we probably wouldn't be in this situation btw.

Backing out that bug would be best IMO but I have no idea if that's possible anymore.

I suspect there isn't Android test coverage for this which is a problem to be solved by the Android team.

Blocks: 1492950
Flags: needinfo?(MattN+bmo)
I'm responsible for this, I'll own it.
Assignee: nobody → mconley
After some doing, I've got a local Fennec build where I'm able to reproduce the issue. Starting investigation...
Maybe I'm doing this all wrong, but debugging our Custom Tabs implementation is a real pain because I can't actually attach WebIDE to an instance of it.

So I used logcat, and here's the interesting part:

11-02 12:14:54.063  7091  7125 E GeckoConsole: [JavaScript Error: "TypeError: tabbrowser.getBrowserForOuterWindowID is not a function" {file: "resource://gre/modules/LoginManagerParent.jsm" line: 306}]
GeckoViewTab stubs out gBrowser on the root window for WebExtension compatibility.
LoginManagerParent also looks for gBrowser on windows that are handling username
and password form fills, and expects it to implement getBrowserForOuterWindowID.

This patch makes the stub implement getBrowserForOuterWindowID, which just returns
the lone GeckoViewTab browser.
Pushed by
Stub out getBrowserForOuterWindowID for GeckoViewTab. r=jchen
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment on attachment 9022250 [details]
Bug 1498233 - Stub out getBrowserForOuterWindowID for GeckoViewTab. r?jchen

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 492950

User impact if declined: Users that have Firefox for Android set as their default browser application won't get "Save Password" dialogs when in Custom Tab mode.

This also probably impacts GeckoView users.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment 0.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): GeckoView needed to implement a method that LoginManagerParent is now relying on. LoginManagerParent assumed anything implementing a gBrowser-like interface would implement this function.

String changes made/needed: None.
Attachment #9022250 - Flags: approval-mozilla-beta?
Comment on attachment 9022250 [details]
Bug 1498233 - Stub out getBrowserForOuterWindowID for GeckoViewTab. r?jchen

custom tabs regression fix, approved for 64.0b9
Attachment #9022250 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed in the latest version of Nightly 65.0a1 (2018-11-09) on Nokia 6 (Android 7.1.1) and OnePlus 5T (Android 8.1.0).
Verified as fixed on 64.0b9 on Sony Xperia Z5 (Android 6.0.1).

Thank you!
You need to log in before you can comment on or make changes to this bug.