1618579 - Text selection caret is misplaced with dynamic toolbar

Status: RESOLVED DUPLICATE of bug 1618582

(GeckoView :: General, defect, P1)

Description

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: testcase.html

Load minimal example in Fenix Nightly, make sure the url bar is not visible and select text, observe text selection carets are shifted up from where they normally should be.

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Screenshot_20200227-081336_Firefox Nightly.jpg

This is a screenshot of what happens.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

:tylin can you take a look?

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

CC more people that might know more about mobile viewport.

make sure the url bar is not visible and select text

This step in comment 0 can be reproduced by

1. Long tap to select any word in the test case.
2. Swipe up to make the dynamic urlbar disappear.

The text in the test case is in a position:fixed; bottom: 20px; element, so this looks similar to bug 1611032.

I've set a string pref logging.AccessibleCaret="verbose" to dump AccessibleCaret's log in adb logcat to see what AccessibleCaret can observe when the dynamic urlbar is being hidden, but there was only TouchStart, TouchMove, and TouchEnd events. No scroll or apz related callbacks are observed, so AccessibleCaret doesn't aware of something being changed, and it needs to update its position.

Hiro or Botond,
Is there any callback that AccessibleCaret can observe when the dynamic urlbar is changing?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

There is. nsPresContext::UpdateDynamicToolbarOffset for the root prescontext gets called during the dynamic toolbar transitions. But please make sure the change for the AccessibleCaret doesn't cause any reflows.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I am curious how Chrome handles AccessibleCaret.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

I don't quite follow AccessibleCaret issue on position:fixed elements, but it's not fixed yet?

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Thanks Hiro! I'll take a look into nsPresContext::UpdateDynamicToolbarOffset, and maybe AccessibleCaret needs to be a resize event observer.

I don't quite follow AccessibleCaret issue on position:fixed elements, but it's not fixed yet?

Bug 1526268 fixed AccessibleCaret's position on position:fixed elements when scrolling the page. But in this bug, when the dynamic urlbar is changing, no scrolling is involved.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

That's good to know. It's probably fixed by bug 1618582. In such cases, the dynamic toolbar should not move in the first place, right? :snorp your patch didn't fix this?

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

That's good to know. It's probably fixed by bug 1618582. In such cases, the dynamic toolbar should not move in the first place, right? :snorp your patch didn't fix this?

Yeah, Fenix has to do some stuff on their end: https://github.com/mozilla-mobile/fenix/issues/8768#issuecomment-592718468

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Thank you snorp! I am going to mark this bug as a dup of bug 1618582.

Comment 11

[Chris Peterson [:cpeterson]]

Moving toolbar bugs to the new GeckoView::Toolbar component.