Remove the context.preselected property and rely on result.heuristic instead
Categories
(Firefox :: Address Bar, task, P1)
Tracking
()
People
(Reporter: adw, Assigned: adw)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
For the nudges experiment, we need to open and show the urlbar view with a single tip. The tip's button should be preselected. See https://mozilla.invisionapp.com/share/S5SQCACBVAF#/screens/370460250_Search_Tips_Experiment
The urlbar webextension API doesn't have a way for extensions to set context.preselected
. (Quantumbar does pass contexts to extensions via the onBehaviorRequested
and onResultsRequested
events. Extensions can set context.preselected
in their event listeners, but that does not work as expected. After calling a listener, context.preselected
remains undefined inside UrlbarProviderExtension
. Adding a preselected
property to the Query
type in the urlbar.json schema doesn't help. I'm guessing that's due to process separation.)
At first I considered adding a preselected
property to the Result
type in urlbar.json. Then extensions could set it just like they can heuristic
. But we always set preselected
and heuristic
together, it looks like. Actually, this is the only place I could find: https://searchfox.org/mozilla-central/rev/d061ba55ac76f41129618d638f4ef674303ec103/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm#240
So I think we should just get rid of context.preselected
and rely on heuristic
instead. afaict we added it close to the beginning of quantumbar development, and it seems we don't really need it. Tests pass locally with this patch. I'll also push to try.
This way extensions can just set heuristic
and then the result will be preselected.
Assignee | ||
Comment 1•5 years ago
|
||
Please see bug 1594622 for a description.
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
Made some changes, so another try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ab86a30ca3e93f568beb6021bdd9064e4f9e7de
Comment 5•5 years ago
|
||
bugherder |
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
Comment on attachment 9107091 [details]
Bug 1594622 - Quantumbar: Remove the context.preselected property and rely on result.heuristic instead
Beta/Release Uplift Approval Request
- User impact if declined: We'd like the option of running quantumbar experiments on 71 that require this patch (bug 1564506, bug 1568594). Otherwise we'd need to wait until 72.
- 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: Bug 1590461 should be uplifted first to avoid conflicts
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This patch does two things: (1) fix a bug in the urlbar webextensions implementation (so that urlbar results from extensions can be the first, pre-selected result in the urlbar view), (2) make a small internal change to urlbar (by removing a redundancy). In both cases, there's no expected impact to the normal outward appearance/behavior of the urlbar. Has tests.
- String changes made/needed:
Comment 7•5 years ago
|
||
Drew, I'd like to wait on your input on the caused regression in bug 1595340 before evaluating an uplift to beta.
Assignee | ||
Comment 8•5 years ago
|
||
I'm not sure why that failure happened. I don't know enough about Fission to guess how it's involved -- but there was only one failure, which isn't a lot of data, so maybe it's not actually Fission specific. I can't reproduce it locally with Fission enabled. It will probably take some time for me to reproduce it on try to figure out what's happening.
My first guess is that there's some timing issue that the test isn't accounting for, and Fission may be making it worse or exposing it in the first place. I doubt that this is a real problem in Firefox, but it would take some investigation to be sure.
I'll investigate.
Assignee | ||
Comment 9•5 years ago
|
||
Comment on attachment 9107091 [details]
Bug 1594622 - Quantumbar: Remove the context.preselected property and rely on result.heuristic instead
Canceling the uplift request because I have higher priority things to work on than bug 1595340, and uplift is more of a nice-to-have anyway.
Updated•5 years ago
|
Description
•