Closed Bug 1509425 Opened 5 years ago Closed 4 years ago

Can not see the comments on hk.yahoo.com

Categories

(Core :: Web Painting, defect, P2)

63 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- verified
firefox66 --- verified

People

(Reporter: johnw.mail, Assigned: mstange)

References

Details

(Keywords: regression, Whiteboard: [webcompat])

Attachments

(2 files)

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
The problem only on Android version, desktop version work fine.
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
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
Attached 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)

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)

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

I added a test to the patch.

Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/519c1045b51e
Undo an optimization that unexpectedly affected hit testing. r=bas
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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+

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

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+

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.