Closed Bug 1367077 Opened 7 years ago Closed 7 years ago

Ensure form fill (passwords, etc) working in GeckoView-based custom tabs and web apps

Categories

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

defect

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: snorp, Assigned: jchen)

References

Details

Attachments

(4 files)

This might not be working because of the underlying JS/XUL changes, so we need to make sure this is working.
Updating the title to include web apps, I don't think there's a need for separate bugs as it will be functionally the same solution for both.
Blocks: 1285858
Summary: Ensure form fill (passwords, etc) working in GeckoView-based custom tabs → Ensure form fill (passwords, etc) working in GeckoView-based custom tabs and web apps
Jim can you take a look?
Assignee: droeh → nchen
Flags: needinfo?(nchen)
Status: NEW → ASSIGNED
Flags: needinfo?(nchen)
Comment on attachment 8906688 [details]
Bug 1367077 - 1. Move startup utility functions into GeckoViewUtils;

https://reviewboard.mozilla.org/r/178416/#review183402
Attachment #8906688 - Flags: review?(snorp) → review+
Comment on attachment 8906689 [details]
Bug 1367077 - 2. Move PromptService startup to BrowserCLH;

https://reviewboard.mozilla.org/r/178418/#review183404
Attachment #8906689 - Flags: review?(snorp) → review+
Comment on attachment 8906705 [details]
Bug 1367077 - 4. Remove LoginManagerParent.login;

https://reviewboard.mozilla.org/r/178442/#review183412
Attachment #8906705 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8906690 [details]
Bug 1367077 - 3. Move form fill event listeners out of browser.js;

https://reviewboard.mozilla.org/r/178420/#review183418

::: mobile/android/chrome/content/browser.js:49
(Diff revision 2)
> -                                  "resource://gre/modules/LoginManagerContent.jsm");
> -
> -XPCOMUtils.defineLazyModuleGetter(this, "LoginManagerParent",
> -                                  "resource://gre/modules/LoginManagerParent.jsm");
> -
>  XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm");

Drive-by comment, while I'm here:

It would probably be a good idea to replace these `defineLazyModuleGetter` calls with a single `defineLazyModuleGetters` call, since the latter uses a JIT-friendly loop, and tends to be much faster. Similar for `defineLazyServiceGetters`.

I already did this with most of the mass lazy module defines in browser/ and toolkit/, but it would probably be good to do the same for mobile/.
Thanks! I filed bug 1399001.
Comment on attachment 8906690 [details]
Bug 1367077 - 3. Move form fill event listeners out of browser.js;

https://reviewboard.mozilla.org/r/178420/#review183844
Attachment #8906690 - Flags: review?(s.kaspari) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/83cf26e44284
1. Move startup utility functions into GeckoViewUtils; r=snorp
https://hg.mozilla.org/integration/autoland/rev/c6300312d42a
2. Move PromptService startup to BrowserCLH; r=snorp
https://hg.mozilla.org/integration/autoland/rev/8df5e093dd92
3. Move form fill event listeners out of browser.js; r=sebastian
https://hg.mozilla.org/integration/autoland/rev/0509b09c11fa
4. Remove LoginManagerParent.login; r=kmag
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3af67a62be1b
1. Move startup utility functions into GeckoViewUtils; r=snorp
https://hg.mozilla.org/integration/autoland/rev/215f47ca940c
2. Move PromptService startup to BrowserCLH; r=snorp
https://hg.mozilla.org/integration/autoland/rev/8a3e6fbd6511
3. Move form fill event listeners out of browser.js; r=sebastian
https://hg.mozilla.org/integration/autoland/rev/6c65c331b97f
4. Remove LoginManagerParent.login; r=kmag
Flags: needinfo?(nchen)
Backed out for failing mochitest-chrome's test_hidden_select_option.html and test_select_disabled.html on Android:

https://hg.mozilla.org/integration/autoland/rev/763af7d6686da5cabb27188dafd70d9ef07ce9f0
https://hg.mozilla.org/integration/autoland/rev/12144eb1c101220118c1d39b32e23ecc1677240e
https://hg.mozilla.org/integration/autoland/rev/5384d984aa5314e80d9a12bffe85e5f036681c4a
https://hg.mozilla.org/integration/autoland/rev/1083f0e1147bb86c69bc3074e5557b10f13d1d7a

Push which ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=84bd9434491c9391a6e961e63e15807224dbeb77&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130708779&repo=autoland

 139 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_hidden_select_option.html | uncaught exception - ReferenceError: XPCOMUtils is not defined at @chrome://browser/content/SelectHelper.js:6:1
160 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_select_disabled.html | uncaught exception - ReferenceError: XPCOMUtils is not defined at @chrome://browser/content/SelectHelper.js:6:1
Flags: needinfo?(nchen)
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/682c9d79073c
1. Move startup utility functions into GeckoViewUtils; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/72e591f41777
2. Move PromptService startup to BrowserCLH; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/17f9dd20314a
3. Move form fill event listeners out of browser.js; r=sebastian
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bb25516598f
4. Remove LoginManagerParent.login; r=kmag
Flags: needinfo?(nchen)
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f01cd7cd431c
Follow-up to fix bustage; r=me on CLOSED TREE
Comment on attachment 8906688 [details]
Bug 1367077 - 1. Move startup utility functions into GeckoViewUtils;

Beta request for all patches.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Custom tabs feature slated for 57 would not have the form/login fill feature.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: Well-tested locally and the patches only just missed the Beta merge by a few hours.
[String changes made/needed]: None
Attachment #8906688 - Flags: approval-mozilla-beta?
Comment on attachment 8906688 [details]
Bug 1367077 - 1. Move startup utility functions into GeckoViewUtils;

This shouldn't need an uplift request to get into 57, since we are going to sync m-c to m-b again.
Attachment #8906688 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Depends on: 1403566
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.