1556774 - User text entry in location / address bar gets replaced with first autocomplete/history/search hit when switching tabs and back

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[:Gijs (he/him)]

STR:

1. add bookmark with "%S" with a keyword
2. open new tab and use the keyword with replacement "foo", load that page
3. open another tab, use keyword and type just "f", do not load page
4. switch to another tab and back

ER:
location bar has "keyword f"

AR:
location bar has full, completed url with replacement "foo"

I think this is a regression with quantumbar but I haven't verified that yet. Reproduced on beta 68b4 (with quantumbar still enabled because early beta).

Comment 1

[:Gijs (he/him)]

Looks like similar things happen with any substring, e.g. after typing just "foo" in a tab and switching away and back to the tab, suddenly the URL bar has "football" (which I assume is a search hit or something).

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I'm having difficulties reproducing this bug, Drew?

Comment 3

[Drew Willcoxon :adw]

(In reply to :Gijs (he/him) from comment #1)

Looks like similar things happen with any substring, e.g. after typing just
"foo" in a tab and switching away and back to the tab, suddenly the URL bar
has "football" (which I assume is a search hit or something).

I can't reproduce it either, and I don't recall it happening in my normal use. When it happens, could you check to see whether the replacement string is one of the results in the popup, like one of the search suggestions? That would be interesting if so. Since neither Marco nor I can reproduce it, do you think you could narrow down the STR any further? Does it happen all the time?

Comment 4

[:Gijs (he/him)]

STR:

1. clean profile on current beta or nightly
2. open prefs/options, turn off "Ctrl+Tab cycles through tabs in recently used order" (this being off used to be the default and existing profiles were never changed)
3. open a new tab
4. type in "foo"
5. accel-shift-tab to go back to the previous tab
6. accel-tab to go to the next tab

ER:
location bar says "foo"

AR:
location bar says "food poisoning" or some other search result

Comment 5

[:Gijs (he/him)]

(The STR in comment #4 reproduce 100% of the time for me. Tested on macOS, if that matters.)

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

On Windows I cannot reproduce yet, so maybe it's a Mac thing? I may try later.

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

oh wait nevermind, I CAN reproduce. I suspect we are actually handling CTRL + TAB as TAB and moving the selection in the urlbar view (setting value)

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

or better, we are handling SHIFT+TAB, moving the selection backwards, that means select the last search suggestion if search suggestions are after results.

Comment 9

[:Gijs (he/him)]

I assume we don't intend to let quantumbar ride with 68 at this point?

This was regressed by bug 1501623 (found via mozregression with quantumbar forced to true)

Comment 10

[:Gijs (he/him)]

(In reply to Marco Bonardo [::mak] from comment #8)

or better, we are handling SHIFT+TAB, moving the selection backwards, that means select the last search suggestion if search suggestions are after results.

Note that TAB has the same effect for me, ie if I have just 2 tabs open, both about:newtab, empty location bar in both, then in one of them type "foo" and then hit ctrl+tab, the bug occurs.

Comment 11

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #10)

(In reply to Marco Bonardo [::mak] from comment #8)

or better, we are handling SHIFT+TAB, moving the selection backwards, that means select the last search suggestion if search suggestions are after results.

Note that TAB has the same effect for me, ie if I have just 2 tabs open, both about:newtab, empty location bar in both, then in one of them type "foo" and then hit ctrl+tab, the bug occurs.

Oh, but I get different values as a result, so that corroborates your hypothesis - we're picking the first suggestion if using accel-tab, and the last suggestion when using accel-shift-tab...

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

yes, we are not considering CTRL in practice and handling the keypress without it.

Comment 13

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs (he/him) from comment #9)

I assume we don't intend to let quantumbar ride with 68 at this point?

Let's be real careful with these kind of statements, since you're proposing to unship a whole feature on this bug only. This may be a considerably serious bug, but I suspect the fix to be trivial. I therefore think it's relatively premature to consider the whole feature to be flawed/ at risk at this point.

I'm requesting tracking for this bug, because we intend to ship QuantumBar with Firefox 68.

Comment 14

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #13)

(In reply to :Gijs (he/him) from comment #9)

I assume we don't intend to let quantumbar ride with 68 at this point?

Let's be real careful with these kind of statements, since you're proposing to unship a whole feature on this bug only. This may be a considerably serious bug, but I suspect the fix to be trivial. I therefore think it's relatively premature to consider the whole feature to be flawed/ at risk at this point.

I didn't intend to propose anything - that's why I asked! - it's just that the last time I checked, quantumbar's pref was ifdef'd on EARLY_BETA_OR_EARLIER, indicating it wasn't set to ride the train. This seems to have changed literally yesterday, and wasn't widely announced (yet), so sorry for not getting the memo... :-\

Comment 15

[Mike de Boer [:mikedeboer]]

:D We're good - apology for assuming you intended otherwise - we indeed didn't announce our plans to ship with 68 widely! I'm quite confident we'll be able to fix this in the most reasonable timeframe. Thanks!

Comment 16

[Dão Gottwald [:dao]]

Here's where legacy code handles this:
https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/toolkit/content/widgets/autocomplete.xml#414

Comment 17

[Dão Gottwald [:dao]]

Attachment: Bug 1556774 - Don't handle Tab key when Ctrl or Alt are pressed. r=Standard8

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc447daeb26f
Don't handle Tab key when Ctrl or Alt are pressed. r=Gijs

Comment 19

[Raul Gurzau (:RaulG)]

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

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 9070775 [details]
Bug 1556774 - Don't handle Tab key when Ctrl or Alt are pressed. r=Standard8

Beta/Release Uplift Approval Request

• User impact if declined: see comment 4
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 4. (I want to look into writing an automated test but think we should uplift this in the meantime.)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): trivial fix ported from the legacy urlbar code
• String changes made/needed:

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 9070775 [details]
Bug 1556774 - Don't handle Tab key when Ctrl or Alt are pressed. r=Standard8

quantumbar fix for 68.0b10

Comment 22

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d31a0e1b47e

Comment 23

[Cristian Craciun]

Build ID 20190613095633
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:69.0) Gecko/20100101 Firefox/69.0

Verified as fixed on the latest Nightly (v69.0a1) on macOS 10.13.

Comment 24

[Cristian Craciun]

Build ID 20190613141208
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Firefox/68.0

Verified as fixed on the latest Beta build (v68.b10) on macOS 10.13.