1638594 - Can't Select / Copy Text on about:support

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Emily Kager [:ekager]]

Originally filed in Fenix as https://github.com/mozilla-mobile/fenix/issues/10653

Steps to reproduce

1. Navigate to about:support page.
2. Long press the page in order to select/copy text.
3. Observe behavior.

Expected behavior

The context menu is displayed and text can be selected/copied/shared.

Actual behavior

The context menu is not triggered, thus text can't be selected/copied/shared.

Device information

• Android device: Huawei P20 Lite (Android 8.0.0) - 1080 x 2280 pixels, 19:9 ratio (~432 ppi density)
• Fenix version: Firefox Preview Nightly 200513 (🦎 78.0a1-20200511094328)

Notes:

1. The issue is not reproducible on Firefox Release 68.8.0
2. Screenshot attached.

Comment 1

[Emily Toop (:fluffyemily)]

We think this is an issue with offset of the actual touch events somewhere.

Comment 2

[Timothy Nikkel (:tnikkel)]

Did it work on Fenix in the (recent) past?

Comment 3

[Botond Ballo [:botond]]

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

Did it work on Fenix in the (recent) past?

Yeah, it looks like it worked in the May 10 nightly and stopped working in the May 11 nightly, suggesting a regression from bug 1556556.

Comment 4

[Botond Ballo [:botond]]

Unsurprisingly, the issue is related to the fact that about:support is rendered in the parent process, so we have a different document structure where the RCD is not the root document of the process. The hit testing rework failed to account for that in some places.

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1638594 - Log the viewport type in FindFrameTargetedByInputEvent(). r=tnikkel

Comment 6

[Botond Ballo [:botond]]

Attachment: Bug 1638594 - Use visual coordinates when hit testing in a chrome document. r=tnikkel

It does not make sense to ask for layout coordinates relative to
a chrome document. If the chrome document has a zoomed (RCD)
descendant in the same process, this means we do not apply the
visual-to-layout transform when later entering the RCD.

Depends on D79588

Comment 7

[Botond Ballo [:botond]]

Attachment: Bug 1638594 - Handle layout coordinates relative to the RCD viewport frame correctly in the GetECRT slow-path. r=tnikkel

Asking for layout coordinates relative to the RCD viewport frame
is a special case because the RCD viewport frame is outside the
zoom boundary, and so should technically always use visual
coordinates.

Nonetheless, we need to use layout coordinates relative to the
RCD viewport frame in several places, as we don't have a frame
that's inside the zoom boundary and sized to the initial
containing block (bug 1641279 may introduce such a frame in the
future).

This means GetEventCoordinatesRelativeTo needs special handling
to apply the visual-to-layout transform when converting from
visual coordinates relative to the root document's viewport
frame, to layout coordinates relative to the RCD viewport frame.

If the RCD is the root document, we take the fast-path which
handled this with an explicit check, but if the RCD is not the
root document, we take the slow-path which does not handle this.
This patch refactors GetECRT so it always checks for this
special case at the end.

Depends on D79589

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5b787706ade
Log the viewport type in FindFrameTargetedByInputEvent(). r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/250602dc0e6c
Use visual coordinates when hit testing in a chrome document. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/1d3eb5f9e1b6
Handle layout coordinates relative to the RCD viewport frame correctly in the GetECRT slow-path. r=tnikkel

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/b5b787706ade
https://hg.mozilla.org/mozilla-central/rev/250602dc0e6c
https://hg.mozilla.org/mozilla-central/rev/1d3eb5f9e1b6

Comment 10

[Julien Cristau [:jcristau]]

Is this safe enough to uplift to beta at this late stage, or can it ride the trains to 79?

Comment 11

[Botond Ballo [:botond]]

Based on my reading of this document, there are going to be upcoming Fenix releases based on 78. I think it's important that about:support be functional in a Fenix release as it's an important troubleshooting tool, so I'm going to request uplift.

Comment 12

[Botond Ballo [:botond]]

Comment on attachment 9156474 [details]
Bug 1638594 - Log the viewport type in FindFrameTargetedByInputEvent(). r=tnikkel

Beta/Release Uplift Approval Request

• User impact if declined: Users are unable to select or copy text on about:support in Fenix
• 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: As in comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is a relatively small fix but the code being touched is tricky, so I would rate it as low to moderate risk. I do think it's an important use case to un-break so I'm recommending uplift. Note that, like the scenario being fixed, any fallout should be limited to mobile (Fenix).

Additional notes:

• I marked this as not verified in nightly because the GeckoView build that contains the fix has yet to appear in a Fenix nightly
• An automated test is planned in bug 1637776, though it may prove trick to write due to about:support being a special (parent process content) page
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9156474 [details]
Bug 1638594 - Log the viewport type in FindFrameTargetedByInputEvent(). r=tnikkel

approved for 78.0b8

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/85d1e4a18a6d
https://hg.mozilla.org/releases/mozilla-beta/rev/13680bc2275d
https://hg.mozilla.org/releases/mozilla-beta/rev/c181b7d40105

Comment 15

[Sorina Florean [:sflorean]]

Verified as fixed on Nightly 06/26 with Xiaomi Redmi Note 8T (Android 9) and LG G7 FIT (Android 8).