Closed Bug 1491724 Opened 2 years ago Closed 2 years ago
Search engine keyword should be highlighted only when the search will actually be performed on the corresponding search engine
14.09 KB, image/png
Bug 1491724 - Search engine keyword should be highlighted only when the search will actually be performed on the corresponding search engine
46 bytes, text/x-phabricator-request
|Details | Review|
596.77 KB, image/gif
13.81 KB, patch
|Details | Diff | Splinter Review|
Configure Firefox as thus: a) Have, say, Google set as the default search engine b) Give a keyword of, say, `w` (without backticks) to the 'Wikipedia (en)' search engine c) Check 'Show search suggestions in address bar results' d) Uncheck everything under 'When using the address bar, suggest:' Not really necessary to configure exactly as above, just that doing all of the above will make the STR easier to follow. FWIW, (d) can be skipped entirely (adapt the STR accordingly), (c) is default, and (a) can be whichever search engine came default for your Firefox. STR: 1. In the address bar, type `w test`. 2. Notice that `w` is highlighted, as expected. Press enter. You are taken to a Wikipedia page, as expected. 3. In the address bar, type `w test` again. 4. Use cursor to scroll down to the second suggestion. AR: i) `w` is still highlighted in the address bar. ii) But the selected entry says 'w test -- Search with *Google*', and has a lens icon instead of Wikipedia's icon iii) If you press enter, the search is indeed performed with Google. ER: `w` should not be highlighted if it is being interpreted as part of the search query to be sent to the default search engine.
Thanks for filing this.
Assignee: nobody → adw
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
My reasoning in https://bugzilla.mozilla.org/show_bug.cgi?id=1484737#c3 was wrong. I assumed that if a result started with a search alias then it had to be a search alias result, but that's clearly not true. This is such a pain to get right, and all the messy logic in this patch reflects that.
Comment on attachment 9011064 [details] Bug 1491724 - Search engine keyword should be highlighted only when the search will actually be performed on the corresponding search engine Marco Bonardo [::mak] has approved the revision.
Attachment #9011064 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/7e5bf501404a Search engine keyword should be highlighted only when the search will actually be performed on the corresponding search engine r=mak
Testing on 64.0a1 (2018-09-30) in WIndows 10, I noticed a small issue: STR: 1. Give a keyword of `w` to the 'Wikipedia (en)' search engine 2. Type `w test` and press Enter. 3. Click on the url bar. AR: `w` is briefly highlighted before the whole url is selected.
Bruce, thank you. I'll file a new bug for that.
Hi, I've reproduced the issue on Release 62.0.2 (64-bit), checked on Nightly 64.0a1 Build ID: 20181002100236 on Win 10 x64, macOS 10.12 and Ubuntu 16.04 and the issue is fixed, the Search engine keyword is highlighted only when the search will actually be performed on the corresponding search engine.
Status: RESOLVED → VERIFIED
[Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1480505 User impact if declined: We want to uplift other bugs related to activity stream and search keywords -- see bug 1496772. We might as well uplift this bug fix, too. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes 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): Been baking on Nightly for 1.5 weeks now, verified, and has automated tests String changes made/needed: None
Attachment #9016493 - Flags: approval-mozilla-beta?
Comment on attachment 9016493 [details] [diff] [review] Beta/63 patch We already shipped our last beta last week and this is not a fix for an identified 63 regression but regular bug work for a feature with non default preferences set that would also require testing on beta from QA too, this doesn't seem like a good candidate for late betas or release, sorry.
Attachment #9016493 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.