1399454 - The address bar should be focused before first paint

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Florian Quèze [:florian]]

Attachment: Screen recording

If we decide to keep focusing the address bar at startup and for new windows (ie. wontfix bug 1395961), we should ensure it's already displayed focused at first paint.

Currently the address bar is first painted unfocused, then it gets focused, and then the down arrow appears with a CSS transition.

See attached screenrecording (to play frame by frame).

Comment 1

[Ed Lee :Mardak]

As per bug 1395961 comment 18, we want focus in the location bar by default. Do we just assume any new window should have focus there by default independent of what first tab gets loaded (e.g., session restore, opening a specific url when launching firefox)? Any idea who should decide?

Comment 2

[Dão Gottwald [:dao]]

(In reply to Ed Lee :Mardak from comment #1)
> As per bug 1395961 comment 18, we want focus in the location bar by default.
> Do we just assume any new window should have focus there by default
> independent of what first tab gets loaded (e.g., session restore, opening a
> specific url when launching firefox)?

I think we can set focus earlier while taking those cases into account. It's currently waiting until after the first paint because that's when we start loading the home page, but the necessariy information about the home page should usually be available before that.

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 1399454 - Set initial focus before the first paint.

Review commit: https://reviewboard.mozilla.org/r/181774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/181774/

Comment 4

[Florian Quèze [:florian]]

(In reply to Ed Lee :Mardak from comment #1)
> As per bug 1395961 comment 18, we want focus in the location bar by default.
> Do we just assume any new window should have focus there by default
> independent of what first tab gets loaded (e.g., session restore, opening a
> specific url when launching firefox)?

Probably not. I don't think focusing the urlbar would make sense when the window is the result of clicking "Open Link in New Window" or when tearing off a tab. I think we'll want to focus the urlbar only when loading the homepage. There's existing code doing a similar check at http://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/browser/base/content/browser.js#1783

I haven't thought about the session restore cases yet.

> Any idea who should decide?

UX (probably :phlsa) if we need a final decision, but that may take a while and we don't have much time left to fix this for 57. So I think we (engineers) should just try to come up with a sensible plan, and request a decision from UX only if we can't reach a consensus.

(I mid-aired with Dão... and it seems his comment and patch are going in the same direction as my comment, so we likely don't need to involve UX here.)

Comment 5

[Dão Gottwald [:dao]]

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

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8910278 [details]
Bug 1399454 - Set initial focus before the first paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/181774/diff/1-2/

Comment 7

[Dão Gottwald [:dao]]

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

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8910278 [details]
Bug 1399454 - Set initial focus before the first paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/181774/diff/2-3/

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8910278 [details]
Bug 1399454 - Set initial focus before the first paint.

https://reviewboard.mozilla.org/r/181774/#review187526

Looks good and worked as expected during my local testing on Mac, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad7b264c22cd
Set initial focus before the first paint. r=florian

Comment 11

[Ed Lee :Mardak]

Attachment: video of fix (focus before paint, included in zoom animation, slowed to 25%)

Comment 12

[Ed Lee :Mardak]

Looks like this just missed the beta merge. The last autoland that made it was bug 1401809 comment 12 03:21 PDT vs here comment 10 06:16 PDT.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 14

[Florian Quèze [:florian]]

Comment on attachment 8910278 [details]
Bug 1399454 - Set initial focus before the first paint.

Approval Request Comment
[Feature/Bug causing the regression]: Fixing this is part of the photon-performance effort to improve perceived performance of startup.
[User impact if declined]: flickering of the urlbar during startup
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: start the browser or open a new window, the urlbar should be focused at first paint and not a couple frames later.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not really
[Why is the change risky/not risky?]: mostly moving some code around to have a focus call done before first paint rather than during delayed startup.
[String changes made/needed]: none

Comment 15

[Sylvestre Ledru [:Sylvestre]]

This is the beginning of the beta cycle, taking it because performance in 57 is our focus!
But I would like someone from QE to verify we didn't regress

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8910278 [details]
Bug 1399454 - Set initial focus before the first paint.

Should be in 57b3

Comment 17

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-beta/rev/658cf4a58da3e48c31e231740258e6785b53c921

Comment 18

[Marco Castelluccio [:marco]]

On my Windows machine, build ID 20170922220129 (after the first build ID where this landed, 20170922100051), the focus is not on the location bar when I start Firefox (it is not in the activity stream search box either). It is on the location bar only when I open a new window.

Comment 19

[Dão Gottwald [:dao]]

(In reply to Marco Castelluccio [:marco] from comment #18)
> On my Windows machine, build ID 20170922220129 (after the first build ID
> where this landed, 20170922100051), the focus is not on the location bar
> when I start Firefox (it is not in the activity stream search box either).
> It is on the location bar only when I open a new window.

Please file a new bug.

Comment 20

[Deac Alin]

Verified on Latest Nightly 58.0a1 on Windows 10 x 64, Windows 7 x 64, Mac OSX 10.12.5 and Ubuntu 16.04
During testing, it was observed that on Windows platforms(7 & 10) the address bar is displayed focused when starting FF, but in fact it's not - when trying to input anything using the keyboard(press any key), nothing happens (cannot write anything in the bar) and the focus seems to be in a different location.

Comment 21

[Dão Gottwald [:dao]]

Deac Alin, we don't reopen bugs for followup issues. Please file a new bug.

Comment 22

[Deac Alin]

(In reply to Dão Gottwald [::dao] from comment #21)
> Deac Alin, we don't reopen bugs for followup issues. Please file a new bug.

Dao Gottwald, I do not consider that this is a followup issue, as the title and purpose of this bug is: "The address bar should be focused before first paint" and per my understanding, this is the Expected behaviour also (as i do not see any other precise Expected or Acceptance Criteria, so this ticket cannot be closed as Verified Fixed), but I understand the process. Therefore I will follow up the open dependancies and reverify the bugs when they are fixed. Thank you

Comment 23

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 57.0a1, build ID: 20170913220121, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 58.0a1, build ID: 20171030103605, and Fx 57.0b13, on Windows 10 x64.