1747575 - ESC key doesn't cancel out Ctrl+E in a New Window

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[zxggoedp]

Steps to reproduce:

Press Ctrl+E
Hit ESC

Actual results:

Nothing happens. You're still in the 'search with a search engine' mode. Search engine's name stays in the URL field.

Expected results:

You exit the 'search with a search engine' mode & go to the 'if it's a link you go to the website, or you search with a search engine' mode. Search engine's name goes away from the URL field.

Comment 1

[Hani Yacoub, Desktop QA]

Managed to reproduce the issue on Windows 10 x64.

Comment 2

[Cristian Baica [:cbaica], Release Desktop QA]

Hello. This is actually the intended behavior since the introduction of the new design for search. The first press of the 'ESC' key actually dismisses the drop-down of the address bar and the second press of the 'ESC' key will dismiss the one'off engine.

Comment 3

[Cristian Baica [:cbaica], Release Desktop QA]

Re-opening the issue. Upon further investigation it looks like the bug IS happening, but only if the user doesn't have any interaction with the address bar before e.g. no clicks inside the bar, or no text input.
Mozregression returned the following pushlog as being the problem here:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f527b152b687694aea2aba0710729aa472ede3cf&tochange=904db8e18e53be836d984c7c99ff6559f323b044

@Drew can you please have a look as I suspect bug 1723158 might be the issue here.

Comment 4

[Drew Willcoxon :adw]

Thanks Cristian. Backspace doesn't work either.

Comment 5

[Drew Willcoxon :adw]

Confirming bug 1723158 as the regressing bug, the problem is here: https://hg.mozilla.org/mozilla-central/rev/904db8e18e53#l4.17

Comment 6

[Drew Willcoxon :adw]

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

Comment 7

[Drew Willcoxon :adw]

Attachment: Bug 1747575 - Fix a bug that prevents search mode from being exited in new windows.

This is a regression from bug 1723158, specifically this change.

this._queryContext is undefined in the view in new windows. If you press the
key shortcut to enter search mode immediately in a new window, search mode is
entered without running a query. Then if you hit Escape or Backspace, we hit the
allowEmptySelection getter and throw an error because this._queryContext is
undefined but we're trying to destructure it. This bug does not happen if you
first focus the urlbar and then enter search mode because focusing the urlbar
causes the top-sites query to run.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9140d44c1498
Fix a bug that prevents search mode from being exited in new windows. r=harry

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 9258437 [details]
Bug 1747575 - Fix a bug that prevents search mode from being exited in new windows.

Beta/Release Uplift Approval Request

• User impact if declined: This is a simple one-line fix that we should ship sooner rather than later.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see bug comments
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-line fix to a JS error, has a test.
• String changes made/needed:

Comment 10

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/9140d44c1498

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Drew Willcoxon :adw from comment #9)

• User impact if declined: This is a simple one-line fix that we should ship sooner rather than later.

I guess we should leave this on the radar for 96 too then.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9258437 [details]
Bug 1747575 - Fix a bug that prevents search mode from being exited in new windows.

Approved for 97.0b3.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6e75dd49cc72

Comment 14

[Drew Willcoxon :adw]

Comment on attachment 9258437 [details]
Bug 1747575 - Fix a bug that prevents search mode from being exited in new windows.

Beta/Release Uplift Approval Request

• User impact if declined: This would be a nice-to-have ride-along on any 96 dot release.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see bug comments
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-line fix to a JS error, has a test.
• String changes made/needed:

Comment 15

[Cristian Baica [:cbaica], Release Desktop QA]

This is verified fixed using Fx98.0a1 and Fx97.0b3 on Windows 10. The ESC key hit now correctly dismisses the address bar and eventually the search mode (on second press). Backspace now correctly removes the engine from the search mode.

Please note that this hasn't made it in the first dot release of 96.

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9258437 [details]
Bug 1747575 - Fix a bug that prevents search mode from being exited in new windows.

Approved by 96.0.2

Comment 17

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/dd275f7d8b43

Comment 18

[Cristian Baica [:cbaica], Release Desktop QA]

The issue is verified fixed using Fx96.0.2 on Windows 10. The search mode can be correctly dismissed using either ESC or BACKSPACE keys.

Comment 19

[Monica Chiorean]

Created TC (https://testrail.stage.mozaws.net/index.php?/cases/view/1656815) for this issue.