1608601 - [Nudges] Urlbar input isn't cleared after clicking the redirect tip's "Okay, Got It" button

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

This is a last-minute nudges/tips bug that Harry found today. It's a problem on 72 and later versions. I don't think it's a show stopper -- we could ship the extension with it -- but I also think we should fix it. We should able to do it entirely from the add-on without landing any production code in m-c, but it will require an experiment API addition.

When the redirect tip is shown (for example on google.com), the input correctly is empty, but when you then click the tip's "Okay, Got It" button, the input's value is set to the page's URL. So you have to clear the input before you can search. It should remain empty when you click the button.

Harry found this while porting the extension in bug 1606909. The value is set to the URL by the handleRevert call here: https://searchfox.org/mozilla-central/rev/d4d6f81e0ab479cde192453bae83d5e3edfb39d6/browser/components/urlbar/UrlbarInput.jsm#615

The stack is:

_setValue (UrlbarInput.jsm#1076)
value (UrlbarInput.jsm#941)
URLBarSetURI (browser.js#3390)
handleRevert (UrlbarInput.jsm#434)
pickElement (UrlbarInput.jsm#615)
_on_mouseup (UrlbarView.jsm#1424)
handleEvent (UrlbarView.jsm#499)

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1608601 - Add browser.experiments.urlbar.clearInput() experiment API.

We need to also set the pageproxystate to invalid so that we show the magnifying glass icon in the input instead of the lock and shield.

This is used in the extension's background script here: https://github.com/0c0w3/urlbar-tips-experiment/pull/8/files#diff-961641dd6af66f5e3313307cf4eb8f1dR339 (Bug 1608613 tracks that PR)

Comment 2

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc5bd95f0400d4b70255afba4d6af65817fbcbb8

Comment 3

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=31c3d59273b64928f1baa7ad54de4270088e804a

Comment 4

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c1f7f7398bcd67a88b8b61dac28277c005a7858

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/119cd111d7d4
Add browser.experiments.urlbar.clearInput() experiment API. r=harry

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/119cd111d7d4

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 9120233 [details]
Bug 1608601 - Add browser.experiments.urlbar.clearInput() experiment API.

Beta/Release Uplift Approval Request

• User impact if declined: No user impact. This patch only adds a test. I'm requesting uplift because it's a test related to a urlbar experiment that we're running on 72, 73, and 74, so it would be good to have test coverage on 73 (and 72, but that ship has sailed).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is only a test
• String changes made/needed:

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9120233 [details]
Bug 1608601 - Add browser.experiments.urlbar.clearInput() experiment API.

Test-only change. Approved for 73.0b5.

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/52fba9ebcb6f