1554158 - 'Choose Download Folder' window is displayed again after changing the download folder and navigating to a different page using enter in the location bar (enter keypress leaks to content from the location bar)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Alin Rus]

Steps to reproduce:

1. Launch Firefox with a new profile.
2. Navigate to about: preferences.
3. Type 'downloads' in 'Find in Options'.
4. Make sure 'Save files to' is checked.
5. Click on 'Browse...'
6. Click on 'New folder'.
7. Type the name for the new folder and hit Enter.
8. Tap on 'Select folder'.
9. From the same window tab, Navigate to 'https://www.thinkbroadband.com/download' page.

Expected results:
https://www.thinkbroadband.com/download page is loaded.

Actual results:
'Choose Download Folder' window is displayed again with https://www.thinkbroadband.com/download page loaded in the background.

Notes:

1. Please note that this is reproducible only when you navigate inside the same tab you've done the changes on downloads folder.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hi Alin,

I assume there's a missing step here, where you have to initiate a download from the https://www.thinkbroadband.com/download page to get the download dialog to appear. Is that correct?

Comment 2

[Alin Rus]

Hi Mike,

You don't have to download some files before, just make sure that after changing the downloads folder, you navigate inside the same tab to a different site, doesn't matter if is https://www.thinkbroadband.com/download .

Comment 3

[:Gijs (he/him)]

I reproduced on Windows in beta. The 'enter' keypress in the location bar when trying to navigate somehow also makes it way to the preferences content in the page and activates the "browse" button again.

(Note: in future, please be specific about how you navigate to a page... if you use a bookmark or your mouse, I don't think this will reproduce)

Alin, how confident are you that this doesn't reproduce on 67? Can you find a regression window?

Comment 4

[Alin Rus]

Hi guys,

Using Mozregression, I've got this:

2019-06-04T16:36:35: INFO : Narrowed inbound regression window from [38a326f8, a8fcad58] (6 builds) to [d1f9cf74, a8fcad58] (2 builds) (~1 steps left)
2019-06-04T16:36:35: DEBUG : Starting merge handling...
2019-06-04T16:36:35: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a8fcad5870bad1bc7d9b355e2b3348753f7385c7&full=1
2019-06-04T16:36:36: DEBUG : Found commit message:
Bug 1548031 - Enable the QuantumBar on Nightly and early Beta. r=adw

Differential Revision: https://phabricator.services.mozilla.com/D29538

2019-06-04T16:36:36: DEBUG : Did not find a branch, checking all integration branches
2019-06-04T16:36:36: INFO : The bisection is done.
2019-06-04T16:36:36: INFO : Stopped

pushlog_url:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=38a326f813f6b1fd1400d215730e0105f62eb9c5&tochange=5cd3bf5c4248c420ffd083f9d1542f4f74963b84

Comment 5

[:Gijs (he/him)]

Alright, seems that the location bar used to swallow the 'enter' keypress and the quantumbar implementation doesn't do this. Hopefully that's easy to fix. Mike/Drew, can you help prio this? Thanks.

Comment 6

[Dão Gottwald [:dao]]

I think this only reproduces with content running in the chrome process. There seem to be two factors to this, the first being whether we consume the key event (i.e. call preventDefault), the second being when we move focus to the content area. I don't see the legacy code consuming the event, so I suspect we move focus earlier. I don't see downsides to just always consuming the event, so I'm going to approach this bug this way.

Comment 7

[Dão Gottwald [:dao]]

Attachment: Bug 1554158 - Always consume the enter key in UrlbarInput. r=Standard8

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e388d4237d0
Always consume the enter key in UrlbarInput. r=mak

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/2e388d4237d0

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 9070774 [details]
Bug 1554158 - Always consume the enter key in UrlbarInput. r=Standard8

Beta/Release Uplift Approval Request

• User impact if declined: see comment 0 (only affects content loaded in the parent process, such as about:preferences)
• 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 0. (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): Always consuming the enter key event in the location bar should be fairly safe, I can't think of possible downsides... (famous last words.)
• String changes made/needed:

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9070774 [details]
Bug 1554158 - Always consume the enter key in UrlbarInput. r=Standard8

fingers crossed, eh? approved for 68.0b10

Comment 12

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/55b21bb55d9e

Comment 13

[Alin Ilea, Desktop QA]

Verified - Fixed on latest Beta 68.0b10 (64-bit) and Nightly 69.0a1 (2019-06-12) on Windows 10 x64.