1398409 - Make FormAssistPopup work for custom tabs

Status: RESOLVED FIXED

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

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1398409 - 1. Move FormAssistant out of browser.js;

To support FormAssistPopup in custom tabs, we need to move the
FormAssitant object out of browser.js and into its own separate file.
BrowserCLH.h in turn loads FormAssistant.js when necessary.

Review commit: https://reviewboard.mozilla.org/r/178448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178448/

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1398409 - 2. Use event callback to communicate FormAssistPopup actions;

Use event callbacks instead of separate events to deliver
FormAssistPopup replies back to FormAssistant. This lets us better
handle having multiple FormAssistPopup instances across Fennec, custom
tabs, and PWAs.

FormAssistant._currentInputElement is removed because it does not allow
us to have multiple concurrent popups. Instead, we track the current
element through the event callback closure.
FormAssistant._currentInputValue is also removed for similar reasons,
and I don't think it was really necessary.

Review commit: https://reviewboard.mozilla.org/r/178450/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178450/

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1398409 - 3. Add FormAssistPopup to custom tabs;

Add and use FormAssistPopup in custom tabs to show autocomplete lists
and form validation messages.

Review commit: https://reviewboard.mozilla.org/r/178452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178452/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8906712 [details]
Bug 1398409 - 1. Move FormAssistant out of browser.js;

https://reviewboard.mozilla.org/r/178448/#review183702

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8906713 [details]
Bug 1398409 - 2. Use event callback to communicate FormAssistPopup actions;

https://reviewboard.mozilla.org/r/178450/#review183704

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8906714 [details]
Bug 1398409 - 3. Add FormAssistPopup to custom tabs;

https://reviewboard.mozilla.org/r/178452/#review184432

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906712 [details]
Bug 1398409 - 1. Move FormAssistant out of browser.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178448/diff/1-2/

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906713 [details]
Bug 1398409 - 2. Use event callback to communicate FormAssistPopup actions;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178450/diff/1-2/

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906714 [details]
Bug 1398409 - 3. Add FormAssistPopup to custom tabs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178452/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a99a2486f70c
1. Move FormAssistant out of browser.js; r=sebastian
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1de7fdaa383
2. Use event callback to communicate FormAssistPopup actions; r=sebastian
https://hg.mozilla.org/integration/mozilla-inbound/rev/d190ba5c8b7f
3. Add FormAssistPopup to custom tabs; r=snorp

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a99a2486f70c
https://hg.mozilla.org/mozilla-central/rev/b1de7fdaa383
https://hg.mozilla.org/mozilla-central/rev/d190ba5c8b7f

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906712 [details]
Bug 1398409 - 1. Move FormAssistant out of browser.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178448/diff/2-3/

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906713 [details]
Bug 1398409 - 2. Use event callback to communicate FormAssistPopup actions;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178450/diff/2-3/

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906714 [details]
Bug 1398409 - 3. Add FormAssistPopup to custom tabs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178452/diff/2-3/

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8906712 [details]
Bug 1398409 - 1. Move FormAssistant out of browser.js;

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 popup / autocomplete 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

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8906712 [details]
Bug 1398409 - 1. Move FormAssistant out of browser.js;

This shouldn't need an uplift request to get into 57, since we are going to sync m-c to m-b again.