Search engine keyword should be highlighted only when the search will actually be performed on the corresponding search engine

VERIFIED FIXED in Firefox 64

Status

()

P1
normal
VERIFIED FIXED
6 months ago
5 months ago

People

(Reporter: bruce.bugz, Assigned: adw)

Tracking

unspecified
Firefox 64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 verified)

Details

Attachments

(4 attachments)

(Reporter)

Description

6 months ago
Posted image Capture.PNG
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.
(Reporter)

Updated

6 months ago
Component: Activity Streams: Newtab → Address Bar
(Assignee)

Comment 1

6 months ago
Thanks for filing this.
Assignee: nobody → adw
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
(Assignee)

Updated

6 months ago
Blocks: 1484737
(Assignee)

Comment 2

6 months ago
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+

Comment 4

6 months ago
Pushed by dwillcoxon@mozilla.com:
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

Updated

6 months ago
Blocks: 1493536
(Reporter)

Comment 5

6 months ago
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.
(Reporter)

Comment 6

6 months ago
https://hg.mozilla.org/mozilla-central/rev/7e5bf501404a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
(Assignee)

Comment 8

6 months ago
Bruce, thank you.  I'll file a new bug for that.
(Assignee)

Updated

6 months ago
Depends on: 1495614
(Assignee)

Updated

6 months ago
Flags: qe-verify+

Comment 9

6 months ago
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
Flags: qe-verify+

Updated

6 months ago
status-firefox64: fixed → verified
(Assignee)

Updated

6 months ago
Depends on: 1497515
(Assignee)

Comment 10

6 months ago
Posted patch Beta/63 patchSplinter Review
[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.