Closed Bug 1638594 Opened 5 months ago Closed 4 months ago

Can't Select / Copy Text on about:support

Categories

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

78 Branch
Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox-esr68 --- unaffected
firefox77 --- unaffected
firefox78 --- fixed
firefox79 --- verified

People

(Reporter: ekager, Assigned: botond)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

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.

Component: General → Panning and Zooming
Product: GeckoView → Core

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

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

(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.

Assignee: nobody → botond
Regressed by: 1556556

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.

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

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

Pushed by bballo@mozilla.com:
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

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

Flags: needinfo?(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.

Flags: needinfo?(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:
Attachment #9156474 - Flags: approval-mozilla-beta?
Attachment #9156475 - Flags: approval-mozilla-beta?
Attachment #9156476 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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

approved for 78.0b8

Attachment #9156474 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9156475 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9156476 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.