1509425 - Can not see the comments on hk.yahoo.com

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P2)

Description

[John Wong]

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

Comment 1

[John Wong]

The problem only on Android version, desktop version work fine.

Comment 2

[Stefan Deiac]

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.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

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

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Over to layout for a look.

Comment 5

[Daniel Holbert [:dholbert]]

Here's a sample URL that triggers this (no need to UA spoof, just go directly to this page):
https://hk.mobi.yahoo.com/home/%E6%B5%B7%E9%82%8A%E8%B1%AC%E5%B1%8D%E8%AD%89%E5%B8%B6%E9%9D%9E%E6%B4%B2%E8%B1%AC%E7%98%9F%E7%97%85%E6%AF%92%E3%80%80%E5%8F%B0%E8%BE%B2%E5%A7%94%E6%9C%83%EF%BC%9A%E6%87%89%E4%BE%86%E8%87%AA%E5%A4%A7-101503726.html

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: 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")

Comment 7

[Daniel Holbert [:dholbert]]

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

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=99869599a2f547ab414a11b3dedf0e9df7aa3666&tochange=1f4c59246bff1c98b0052d4edfa4c49d05c52b1a

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

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:

• FilterSupport::ComputePostFilterExtents
• nsFilterInstance::ComputePostFilterExtents
• nsFilterInstance::GetPostFilterBounds

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.

Comment 10

[Bas Schouten (:bas.schouten)]

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.

Comment 11

[Markus Stange [:mstange]]

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.

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1509425 - Undo an optimization that unexpectedly affected hit testing. r?Bas

Comment 13

[Markus Stange [:mstange]]

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

Comment 14

[Markus Stange [:mstange]]

I added a test to the patch.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/519c1045b51e
Undo an optimization that unexpectedly affected hit testing. r=bas

Comment 16

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/519c1045b51e

Comment 17

[Markus Stange [:mstange]]

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:

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/96e7b3aac8ac

Comment 20

[miralobontiu]

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

[miralobontiu]

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

Comment 22

[John Wong]

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