1538050 - Bookmark searches containing question mark (?) appear as the second address bar suggestion

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Tyler Szabo]

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

Steps to reproduce:

Create a bookmark as follows:

1. Find a search input on a page (Bugzilla will be used for this example) and select "Add a Keyword for this Search..." or select "New Bookmark..." from the Bookmark menu and add a location such as "https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%s".
2. In the Keyword field enter a string that ends in a question mark, such as "bugz?"

Attempt to use the search keyword:

Now either in the current tab or a new tab focus the address bar and begin typing "bugz? abc" using the keyword selected in the bookmark creation stage.

Background:

This is a regression: bookmarks with keywords ending in a question mark behaved like a search keyword prior to 66.0.

Workaround:

Search keywords using other special characters such as "colon" and and "at symbol" still behave correctly, so keywords such as "bugz?" can be changed to "@bugz" or "bugz:".

Actual results:

The suggested navigation shows "http://bugz/? abc" and selecting it will attempt to navigate to "http://bugz/? abc" and fall back to the default search engine upon failure.

Expected results:

The suggested navigation shows "bugzilla.mozilla.org: abc" and selecting it will navigate to "https://bugzilla.mozilla.org/buglist.cgi?quicksearch=abc"

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0
20190314174725

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d11bb3bd81951e0e5a66bc9e3ae80ca22d2ad13f&tochange=f2f4a6eb1576cba580ee99971f5fc035cbaeeab1

(In reply to Tyler Szabo from comment #0)

The suggested navigation shows "http://bugz/? abc"

... as the first suggestion. The search bookmark appears as the second suggestion. This makes it necessary to press the Down Arrow key before pressing Enter to submit the query.

Comment 2

[Marco Bonardo [:mak]]

The ? and @ ideally should not be allowed in keywords, iirc we have another bug where we would like to enforce avoiding reserved chars... Anyway, at a maximum I'd expect "word?" to search for "word" and "word? something" to actually work... The tokenizer is likely interpreting this wrongly.
It's an edge case, but we should handle it properly.

Comment 3

[Marco Bonardo [:mak]]

The bug I was speaking about in comment 2 is bug 1517140

Comment 4

[Marco Bonardo [:mak]]

With the new tokenizer, restriction tokens are handle specially if they are at the beginning or end of the search string, regardless of a space after/before them. This was done on purpose to simplify enforcing searches (or other kind of results) through the address bar.

This means I can only add back support for keywords ending with a restriction token IF there is a search string after them, so that some of the legacy keywords will continue working. Other cases are more complicate to support if the tokenizer can't tell if a token is a keyword. See https://bugzilla.mozilla.org/show_bug.cgi?id=1517140#c9
The best path forward is to fix bug 1517140, once QB is the default implementation.

To sum up the expected outcome:

1. "mykeyword? search terms" searches "search terms" with the "mykeyword?" bookmark keyword
2. "mykeyword?" searches "mykeyword" on the default search engine
3. "?mykeyword search terms" searches "mykeyword search terms" on the default search engine
4. "?mykeyword" searches "mykeyword" on the default search engine

Comment 5

[Marco Bonardo [:mak]]

Attachment: Bug 1538050 - Split restriction characters only if they are at the beginning or end of the search string. r=adw!

The new tokenizer, to make more natural typing restrictions like "?search terms", "search terms?" or "%my tab",
splits out restriction characters if they appear at the beginning or end of the search string. This means
keywords and aliases can't in general begin or end with a restriction character, but we don't have an input
check to prevent those.
For now the tokenizer can't recognize keywords or aliases, because it can't be made asynchronous until the
Quantum Bar is the default implementation, and anyway the complexity must be considered at that point.

The best we can do is to stop splitting out restriction character in the middle of the search string.
This will allow a part of the old interaction like searching for "keyword? something".

This patch also prevents us from splitting a %encoded string, fixing bug 1538715.

Comment 6

[Tyler Szabo]

Thank you for clarifying in Comment 4 - that is the behavior I was expecting and what I remember the old behavior to be. However, it's also understandable to disallow the token altogether even though it'll change workflows (relevant: https://xkcd.com/1172/).

I'll add a comment to bug 1517140 as well to account for the case of a build-to-build upgrade changing parsing to somehow notify the user that a keyword that used to work is no longer valid.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1be80f222f5
Split restriction characters only if they are at the beginning or end of the search string. r=adw

Comment 8

[Andrei Ciure[:aciure]]

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