Can not see the comments on hk.yahoo.com

VERIFIED FIXED in Firefox 65

Status

()

defect
P2
normal
VERIFIED FIXED
7 months ago
5 months ago

People

(Reporter: johnw.mail, Assigned: mstange)

Tracking

({regression})

63 Branch
mozilla66
ARM
Android
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 wontfix, firefox64 wontfix, firefox65 verified, firefox66 verified)

Details

(Whiteboard: [webcompat])

Attachments

(2 attachments)

Reporter

Description

7 months ago
User Agent: Mozilla/5.0 (Android 8.1.0; Mobile; rv:63.0) Gecko/63.0 Firefox/63.0

Steps to reproduce:


When browsing hk.yahoo.com, clicking on the "Comments" link to expand the hidden comments element, does not work
Reporter

Comment 1

7 months ago
The problem only on Android version, desktop version work fine.

Comment 2

7 months ago
Hi, thanks for your report. I can confirm the issue on the latest version of Nightly 65.0a1 (2018-11-25) and Release 63.0.2 with Nexus 5 (Android 6.0.1), Nokia 6 (Android 7.1.1) and OnePlus 5T (Android 8.1.0). 
Note: On Chrome is not reproducible.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → ARM

Updated

6 months ago
Product: Firefox for Android → GeckoView
Version: Firefox 63 → 63 Branch
Suspect webcompat. Can reproduce on Firefox for Android, reference browser, and Firefox desktop in responsive design mode with a Firefox for Android UA.

Clear STR:
* Load https://hk.yahoo.com
* Tap on one of the non-sponsored news links at the bottom
* wait for the page to load and attempt to tap on the comments icon on the bottom left
Flags: needinfo?(miket)
Over to layout for a look.
Component: General → Layout
Product: GeckoView → Core
Posted file testcase 1
They have a button with "filter:opacity(0)" which they are assuming will receive click events.

But apparently, in Firefox, it does not receive click events. Here's a reduced testcase, with the problematic CSS inside the red border, and with two other non-problematic examples in the black borders (a nonzero filter:opacity() value, and an explicit "opacity:0")
mstange (when you're back), I think you touched filter code not too long ago; do you know why filter:opacity(0) vs. opacity:0 might have different behavior here, with respect to clickability/hit-targeting?  (And does this seem like a Firefox bug?)
Flags: needinfo?(mstange)
Flags: needinfo?(miket)
Whiteboard: [webcompat]
Priority: -- → P2
Component: Layout → Web Painting

I pretty much believe the bug is on this code: https://searchfox.org/mozilla-central/rev/76fe4bb385348d3f45bbebcf69ba8c7283dfcec7/gfx/src/FilterSupport.cpp#1605-1607 and some other match functions around it can have the same problem as well.

The reason this causes the issue is that it is called in the following chain:

Then either nsSVGIntegrationUtils::ComputePostEffectsVisualOverflowRect or nsSVGUtils::GetPostFilterVisualOverflowRect may be used by hit check.

I guess either the code mentioned above should be fixed, or somewhere in-between should combine the post-effect overflow with the actual content rect when doing hit check.

Hrm, this could be, this was an optimization I naively added, without knowing this type of filter code was also used for hittesting. Mstange is the right person to look at this and point out how the optimization could be moved.

Flags: needinfo?(bas)
Assignee

Comment 11

5 months ago

I guess either the code mentioned above should be fixed, or somewhere in-between should combine the post-effect overflow with the actual content rect when doing hit check.

I agree. I filed bug 1520553 about that.

To get this regression from 63 fixed quickly, I think we should just remove the opportunistic optimization - that would be a simple patch and it could be uplifted without much risk. The better fix can then go into bug 1520553 and ride the trains normally.

Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
Assignee

Comment 13

5 months ago

I've confirmed that this patch fixes attachment 9034239 [details] in a local Mac build.

Assignee

Comment 14

5 months ago

I added a test to the patch.

Comment 15

5 months ago
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/519c1045b51e
Undo an optimization that unexpectedly affected hit testing. r=bas

Comment 16

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee

Comment 17

5 months ago

Comment on attachment 9037412 [details]
Bug 1509425 - Undo an optimization that unexpectedly affected hit testing. r?Bas

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1473937

User impact if declined: Clicking some elements on some pages doesn't work (pages that expect filter:opacity(0) elements to be clickable)

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment 3 or comment 5

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Extremely low risk, the patch is just a partial backout of bug 1473937 and adds a test.

String changes made/needed:

Attachment #9037412 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9037412 [details]
Bug 1509425 - Undo an optimization that unexpectedly affected hit testing. r?Bas

[Triage Comment]
Partially reverts an optimization which ended up causing some web-facing issues. Adds a new automated test. Approved for 65.0 RC2.

Attachment #9037412 - Flags: approval-mozilla-beta? → approval-mozilla-release+

Comment 20

5 months ago

Verified as fixed on the latest Nightly 66.0a1, tested with Nexus 5 (Android 6.0.1), Nokia 6 (Android 7.1.1), and OnePlus 5T (Android 8.1.0).

Comment 21

5 months ago

Verified as fixed on Beta 66.0b3 with Nokia 6 (Android 7.1.1), and OnePlus 5T (Android 8.1.0).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Reporter

Comment 22

5 months ago

Hi, I am original bug reporter.
Yes, I comfire this bug fixed, after upgrade to 65.0 release on android.
Thanks.

You need to log in before you can comment on or make changes to this bug.