1485746 - Cursor gets reset to start of address bar, when I alt-tab away from Firefox and back

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Type "a.b" into URL bar (or any string that involves some .suffix)
 2. Alt-tab to another window.
 3. Alt-tab back to Firefox.

EXPECTED RESULTS:
The cursor should be where I left it -- after "b", at the end of my entered text.

ACTUAL RESULTS:
The cursor has inexplicably been reset to the start of the address bar.


This is quite annoying if you're typing in an address, briefly alt-tab away to check your memory, and then alt-tab back and resume typing.  Your final characters end up at the beginning of the URL, because the cursor position was unexpectedly reset.

This is a recent regression in 63:
10:29.80 INFO: Last good revision: b4ac908007b9af47907b57891fc537201ef489a4
10:29.80 INFO: First bad revision: b6233f4d899c3da8143461b19201baf69e63595f
10:29.80 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b4ac908007b9af47907b57891fc537201ef489a4&tochange=b6233f4d899c3da8143461b19201baf69e63595f

Comment 1

[Daniel Holbert [:dholbert]]

(Wanted to tag mak for needinfo, but it looks like he's not accepting needinfo requests at the moment - hence, tagging Gijs as reviewer of regressing bug for the moment.)

Comment 2

[:Gijs (he/him)]

It's because we set selectionStart = selectionEnd = 0 when the location bar is blurred, in formatValue: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#586 - and 'blur' fires when the window loses focus, apparently.

Off-hand, I can't think of a good way of fixing this. We could check if focus is somewhere else in the document and/or if our window is still active, but then we'd also need a handler that notices when the window regains focus that then checks if the url bar is receiving focus again, because otherwise the focus could just not go back to the url bar and the url could remain scrolled out of view.

I'm actually surprised this wasn't broken before...

Comment 3

[:Gijs (he/him)]

Ugh, wrong mid-air button.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: screencast 1 (of me trying to type "aaa.bbbb", with an alt-tab away & back in the middle)

Here's me trying to type aaa.bbbb into my URL bar, with a brief alt-tab away and back partway through.

This bug makes me end up with "bbbaaa.b" in the url, rather than "aaa.bbbb"

Comment 5

[:Gijs (he/him)]

The point here is that we don't want to end up with a situation where people can be misled into a situation where they end up with a formatted URL bar that isn't showing the host. formatValue() is only called when we stop editing the URL bar, and in that case the host should be visible. That's what the code is doing, and I don't see a particularly good way to disambiguate the STR from comment #0 from ones where the location bar *actually* loses focus, as it were. :-(

Comment 6

[Drew Willcoxon :adw]

Maybe we could save the caret position on blur and then restore it on focus?  Some details to work out, obviously, but?

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked as a recent regression in 63, I was also able to repro this on Nightly63.

Comment 8

[ovidiu boca[:Ovidiu]]

I reproduced it on Mac OS X 10.12 with Nightly (2018-08-23).

Comment 9

[Drew Willcoxon :adw]

... or just set scrollLeft to 0?  Seems to work...

Comment 10

[Drew Willcoxon :adw]

Attachment: Bug 1485746 - Cursor gets reset to start of address bar, when I alt-tab away from Firefox and back

Comment 11

[Drew Willcoxon :adw]

Well now that I post that, I realize it's not perfect because when you switch back to Firefox, the text has scrolled back to the host and the caret isn't visible.  But, the caret is still actually where you left it at least, and when you start typing or moving with the arrow keys, the text jumps to where the caret is.

Comment 12

[:Gijs (he/him)]

We used to only manipulate the scroll/cursor position when the value changed, prior to bug 1419391 landing. Is it not possible to do something similar again, instead of this workaround (which as comment #11 notes, still has some issues) ? Perhaps we can detect if the value has changed before making a decision, or if the blur is a result of "submission" (ie hitting enter / go / paste and go / drag+dropping a URL) in the url bar, or something? Would that be too fragile? Maybe Marco can clarify why he implemented it the way he did?

I'm out tomorrow (it's a public holiday here), so I won't have time to dive into this properly until Tuesday, but naively I would hope we could come up with a better fix. That said, we're doing the final merging to beta in just over a week, and I think we should land the patch Drew attached if we can't come up with something better before then (we can always uplift still-better fixes if we come up with them, something's better than nothing etc.).

Comment 13

[Marco Bonardo [:mak]]

(In reply to :Gijs (he/him) from comment #12)
> Perhaps we can detect if the value has changed before
> making a decision, or if the blur is a result of "submission" (ie hitting
> enter / go / paste and go / drag+dropping a URL) in the url bar, or
> something? Would that be too fragile? Maybe Marco can clarify why he
> implemented it the way he did?

IIRC it was just matter of coherency, before the selection change was spread around in various call points, I wanted to consolidate it and it seemed to make sense and be safer to have a reliable behavior in general, so when the urlbar is blurred the host (or what is recognized as the host) is always visible.

Drew's approach sounds like a possibility, we already have some jumps of the text for RTL... As you suggested we could also skip the selection set in certain cases. I'll do some testing and see.

Comment 14

[Marco Bonardo [:mak]]

I think in the end I will just restore the previous code touching the selection on change, it's not strictly necessary to keep it.
I must note the problem Drew pointed out in comment 11 is likely to already exist for RTL where we indeed set scrollLeft regardless of the cursor position... But that's likely an edge case anyway and it has more implications with the security fix.

Comment 15

[Marco Bonardo [:mak]]

Attachment: Bug 1485746 - Cursor gets reset to start of address bar on window switch.

This restores the previous behavior where we set the selection only when setting a new different value

Comment 16

[Drew Willcoxon :adw]

Comment on attachment 9004837 [details]
Bug 1485746 - Cursor gets reset to start of address bar on window switch.

Drew Willcoxon :adw has approved the revision.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/621979b26f69
Cursor gets reset to start of address bar on window switch. r=adw

Comment 18

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/621979b26f69

Comment 19

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.12, Windows 10 x64 and Ubuntu 16.04 with FF Beta 63.0b3  and FF Nightly 64.0a1(2018-09-05) and I can confirm the fix.