1504552 - Typing something like `* @wikipedia test` has unexpected behaviour

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Bruce]

Attachment: Capture.PNG

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

Type any[1] of the special characters that filter address bar suggestions, followed by a space, followed by a search engine keyword. So something like `* @wikipedia`.

[1]https://support.mozilla.org/en-US/kb/awesome-bar-search-firefox-bookmarks-history-tabs#w_changing-results-on-the-fly_2


Actual results:

1. @wikipedia was highlighted when it probably shouldn't be

2. First suggestion was:
[wiki favicon] a -- Search with Wikipedia (en)
So the last character of the search engine keyword is being assumed to be part of the search query. For `* @amazon test`, I get:
[amazon favicon] n test -- Search with Amazon.com

3. If I press enter, the browser will indeed do what the first suggestion said, e.g. search Wiki for the letter A.

Comment 1

[Marco Bonardo [:mak]]

sigh, we should consider search shortcuts only if the string begins with them :(

Comment 2

[Drew Willcoxon :adw]

It looks like we've always (accidentally?) been treating "* alias query", "% alias query" etc. as a search engine alias search, and also we've always (or at least for a while) been incorrectly parsing out the query string in that case (because we're taking the substring of the trimmed original search string, which includes the restriction token).  It's more obvious now that we highlight aliases though.

I'm bumping into this while working on bug 1504847 so I think I'll go ahead and fix it now.

Comment 3

[Drew Willcoxon :adw]

I'll handle the highlighting part in bug 1504847 -- i.e., not highlighting built-in @ aliases when the input doesn't start with them.

Comment 4

[Drew Willcoxon :adw]

Attachment: Bug 1504552 - Correctly handle search engine aliases and other keywords when restriction characters are present.

Comment 5

[Drew Willcoxon :adw]

On second thought I should just fix that here too.  I think the decodeURIComponent() in the formatter is wrong and unnecessary also.

Comment 6

[Drew Willcoxon :adw]

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

Comment 7

[Marco Bonardo [:mak]]

I am not totally convinced that in the "* @alias" case we should recognize the alias, I really thing the trimmed original search string should start with the @alias for it to be effective. What's the point of recognizing the alias, since the user first of all asked to limit the query to bookmarks?
What are the remaining results, if they exists? I'd expect them to be bookmarks matching the "@alias" string.
There's an edge case for "? @alias" though...

Comment 8

[Marco Bonardo [:mak]]

And thinking more about it, I think "? @alias" should search "@alias" in the default search engine.

Comment 9

[Marco Bonardo [:mak]]

Plus the added complication we are introducing when the thing is recognized as an alias but not hilighted as one. It all sounds like complication that will hit us in the future. That's why I'm suggesting that the trimmed original search string must start with @, if it doesn't then it's not an alias (or a command), it's just a plain string.

Comment 10

[Drew Willcoxon :adw]

The patch isn't adding any new recognition, see comment 2.  Do you really want to fix that as part of this bug?

Comment 11

[Marco Bonardo [:mak]]

(In reply to Drew Willcoxon :adw from comment #10)
> The patch isn't adding any new recognition, see comment 2.  Do you really
> want to fix that as part of this bug?

I don't see why we should complicate the code and behavior, just to simplify it shortly after...

Comment 12

[Drew Willcoxon :adw]

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

Comment 13

[Drew Willcoxon :adw]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e39e7ada574f
Correctly handle search engine aliases and other keywords when restriction characters are present. r=mak

Comment 15

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e39e7ada574f

Comment 16

[Sayed [:LiquidO2]]

I can reproduce this issue in Nightly 65.0a1 (2018-11-04) (64-bit) on Linux x86_64

I can verify that this is fixed in latest Nightly

Build ID 	20181129095546
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0

Comment 17

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 65.0a1 (2018-11-08) on Windows 7, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20181126100057
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

[bugday-20181128]

Comment 18

[Sayed [:LiquidO2]]

As per comment 16 and comment 17, I am marking this bug as verified.