1549292 - Web content on Twitter goes out to lunch (due to selected fixed-pos content and its just-offscreen AccessibleCaret causing a viewport expansion feedback loop)

Status: NEW

(Core :: Layout, defect, P5)

Description

[csheany]

User Agent: Mozilla/5.0 (Android 7.1.1; Tablet; rv:68.0) Gecko/68.0 Firefox/68.0

Steps to reproduce:

1. Always enable zoom
2. Open https://mobile.twitter.com/FirefoxDevTools/status/1100852277911273472/photo/1
3. Tap

Actual results:

See the attached video

Expected results:

To be determined

Comment 1

[csheany]

Attachment: 2019_05_06_04_15_30.mp4

Comment 2

[miralobontiu]

Hi,

I wasn't able to reproduce the issue on the latest Nightly 68.0a1 (2019-05-02) with Samsung Galaxy Tab S3 (Android 8.0).
When I tapped, the image was displayed as expected.

Thanks,

Comment 3

[csheany]

Did you always enable zoom?

Comment 4

[csheany]

It seems the link opens the thread instead of the image. (Bug)

The issue is with the lightbox view.

Comment 5

[csheany]

Slightly modified STR...

1. Always enable zoom
2. Open https://mobile.twitter.com/FirefoxDevTools/status/1100852277911273472/
3. Tap the link
4. Tap the image

Comment 6

[miralobontiu]

Attachment: 2019_05_08_08_59_08_Trim.mp4

Hi, csheany

I did the STR from Comment 5, and still no issue: the image is displayed, it doesn't move/glitch.
Please let me know if I`m missing something.

Thanks!

Comment 7

[csheany]

STEP 4!

It's not going to do it by itself :)

Comment 8

[miralobontiu]

Hi, again!

I was able to reproduce the issue now, on the latest Nightly 68.0a1 (2019-05-08) and on Beta 67.0b18, with:

• Samsung Galaxy Tab S3 (Android 8.0),
• Huawei Honor 8 (Android 7.0),
• Samsung Galaxy Note 8 (Android 9.0).
  I will confirm the issue.

Not reproducible on RC 66.0.5.

Also, found a regression range:

Last good revision: 4b74d76e55a819852c8fa925efd25c57fdf35c9d
First bad revision: 96d77e9c3f2985c2b702c0910e0da686bee54105

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4b74d76e55a819852c8fa925efd25c57fdf35c9d&tochange=96d77e9c3f2985c2b702c0910e0da686bee54105

Comment 9

[Hiroyuki Ikezoe (:hiro)]

This is somewhat related to bug 1529892. But I have no clue yet, twitter tries to do something to make the layout viewport bigger in response to touch events. If I set dom.w3c_touch_events.enabled to 0 (disabled), the issue disappears.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

CCing :dholbert since this is somewhat related to flex box and position:fixed.

Comment 11

[Daniel Holbert [:dholbert]]

Note:

(1) Even with "always enable zoom" turned off (the default), the tap on the image here still triggers an unexpected context menu (which for me says "Copy|Share|Select All|Google search"). That is a bug and may somehow be the root of the problem here.

(2) If you tap with two fingers (as if to pinch-zoom), or tap-and-drag (even though nothing is usefully draggable at the default zoomlevel), then the issue does not happen. (No context menu, no wacky zooming.)

Comment 12

[Daniel Holbert [:dholbert]]

Two more observations:
(3) Even with "always enable zoom" turned off (the default), the tap-on-the-image action triggers visible scrollbars -- i.e. there's some content appearing outside of the viewport! If you scroll a bit, you can see that this outside-the-viewport content is the Accessible Caret (little blue pointed selection-circles).

(4) If you turn off pref layout.accessiblecaret.enabled_on_touch, then this bug seems to entirely go away. No scrollbars appear, and I can't reproduce the surprise context menu or the insane zooming.

So my current theory is:

• Tapping the image triggers a "selection" action with the accessible caret, for some reason (maybe in part due to how Twitter is handling touch events per comment 9).
• That "selection" action selects a 100%-sized fixed-pos element (covering the full viewport), using the accessible caret, and this creates content (the begin-selection/end-selection carets themselves) outside of the visible part of the viewport.
• If always-enable-zoom is turned on, then this apparently expands the fixedpos element's containing block (by expanding the layout viewport, or something?), which grows the height:100%;width:100% fixed-pos element itself.
• That, in turn, moves the selection-caret pointy-circles outward, which then grows the viewport more, etc etc. And we have a deathspiral outwards.

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1 (100%-sized fixed-pos element)

Comment 14

[Daniel Holbert [:dholbert]]

STR with reduced testcase 1 (included in the testcase, just posting here for clarity):

1. Load testcase.
2. Long-press the page.
3. Choose "Select All"
   (Note: no need to trigger always-enable-zoom; I'm using a more permissive meta viewport than twitter does.)

EXPECTED RESULTS:
Nothing much, just some text selection.

ACTUAL RESULTS:

• the bottom right text disappears offscreen.
• the top left text unwraps to a single line.
• Both of these are a signal that the .fixedPos element has just suddenly gotten huge (via offscreen accessible-caret-thumbs expanding the size of its underlying viewport/containing-block)

Comment 15

[Botond Ballo [:botond]]

Ouch!

(In reply to Daniel Holbert [:dholbert] from comment #12)

• That "selection" action selects a 100%-sized fixed-pos element (covering the full viewport), using the accessible caret, and this creates content (the begin-selection/end-selection carets themselves) outside of the visible part of the viewport.

Ting-Yu, is there ever a situation where selecting an element covering the entire viewport is desirable? If not, adding a guard for this might be an easy way to break the cycle.

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to Botond Ballo [:botond] from comment #15)

Ting-Yu, is there ever a situation where selecting an element covering the entire viewport is desirable? If not, adding a guard for this might be an easy way to break the cycle.

That won't do it. Simply selecting an element at the extreme bottom-right of the viewport is sufficient to trigger this bug.

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2 (fixed-pos selectable text at bottom-right corner)

Comment 18

[Daniel Holbert [:dholbert]]

With reduced testcase 2, when I long-press the text at bottom right to select it, that text gets scrolled way offscreen (because the accessiblecaret creates more scrollable overflow, and bottom:0 and right:0 change meaning [increasing their resolved values] as a result).

To solve this, I think we have to prevent the accessiblecaret from creating scrollable overflow. I see two options for that (and we could even do both):
(a) change the caret-thumb's orientation (e.g. placing it to the top-left of the boundary rather than bottom-right) when it's near the edge of the viewport.

...or:

(b) drawing it with some special rule applied so that it doesn't create scrollable overflow even though it's outside the range of a scrollable area's viewport.

Comment 19

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

Re comment 16:

Ting-Yu, is there ever a situation where selecting an element covering the entire viewport is desirable? If not, adding a guard for this might be an easy way to break the cycle.

I don't think there's a situation we'd want to select an element that covering the entire viewport.

Comment 20

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

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #19)

Re comment 16:

Ting-Yu, is there ever a situation where selecting an element covering the entire viewport is desirable? If not, adding a guard for this might be an easy way to break the cycle.

I don't think there's a situation we'd want to select an element that covering the entire viewport.

I mean it's rare in a real webpage that the user is long-tapping to select an element that coverts the entire viewport. Of course, as dholbert points to me on IRC, "select-all" is a viable action to select such an element like in test case 1 in comment 13.

Also, there's a separate issue in the original twitter STR. The user is just single tapping on the image. AccssibleCaret doesn't have any special logic to handle single tapping, it just update the blue caret handles in AccessibleCaretManager::OnSelectionChanged callback. So we also need to investigate why a single tapping create a non-collapsed selection range for that image.

Comment 21

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

NI myself to dig into this issue later.

Comment 22

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #20)

Also, there's a separate issue in the original twitter STR. The user is just single tapping on the image.[...] So we also need to investigate why a single tapping create a non-collapsed selection range for that image.

I spun off bug 1550869 on that part.

Comment 23

[Hiroyuki Ikezoe (:hiro)]

Thank you, Daniel! Awesome! And thank you, Ting-Yu for taking over this. :)

(In reply to Daniel Holbert [:dholbert] from comment #12)

• If always-enable-zoom is turned on, then this apparently expands the fixedpos element's containing block (by expanding the layout viewport, or something?), which grows the height:100%;width:100% fixed-pos element itself.

I can answer only this. We don't expand the layout viewport if user-scalable=no is specified but it's overridden by the always-enable-zoom pref.

Comment 24

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

The original twitter test case has been fixed in bug 1550869, but the reduced testcase 2 in Comment 17 can still reproduce the crazy zooming behavior. Possible solutions in Comment 18 are still valid.

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 26

[csheany]

STR

Open https://firefox-ci-tc.services.mozilla.com/tasks/index/mobile.v3.firefox-android.apks.fenix-nightly.latest/arm64-v8a
Select All

Actual Results

See the attached video

Expected Results

To be determined

Comment 27

[csheany]

Attachment: Screen_Recording_Firefox Nightly.mp4