Closed Bug 1632941 Opened 5 years ago Closed 5 years ago

Unable to dismiss subscription popup on jevaisvouscuisiner.com

Categories

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

Product:
Core
Component:
Panning and Zooming
Platform:
Unspecified
Android
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- verified

People

(Reporter: ksenia, Assigned: kats)

References

(Regression,
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1632941 - Treat elements with cursor:pointer CSS as event fluffing targets. r?emilio
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta-
Details | Review

As reported here https://github.com/webcompat/web-bugs/issues/51690

Steps to reproduce:

  1. In Firefox Preview open https://jevaisvouscuisiner.com/griesknepfle-gnocchi-semoule-alsace-recette/09/2017/
  2. Wait for the subscription popup to appear (a modal window with text "Abonnez-vous à notre newsletter")
  3. Attempt to close the popup

Expected result:
Popup closes

Actual result:
Popup stays open

From inspecting the click event can see that an element underneath the close button receives the click

From mozregression:

12:42.25 INFO: Narrowed inbound regression window from [ff438277, ede88d07] (3 builds) to [20942931, ede88d07] (2 builds) (~1 steps left)
12:42.25 INFO: No more inbound revisions, bisection finished.
12:42.25 INFO: Last good revision: 209429314dd4548b7158244c501ff31e07d5d525
12:42.25 INFO: First bad revision: ede88d07c59a17a04ce72969b15933506a0f347d
12:42.25 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=209429314dd4548b7158244c501ff31e07d5d525&tochange=ede88d07c59a17a04ce72969b15933506a0f347d

Kartikaya, would you be able to take a look?

Flags: needinfo?(kats)

Yup, I can investigate, thanks!

Assignee: nobody → kats
Flags: needinfo?(kats)

It seems the event handler is on the <body> instead of the <span>, and there isn't even a :hover rule or something so bug 1192558 wouldn't even help... Maybe we should check for cursor: pointer in this case or such?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

It seems the event handler is on the <body> instead of the <span>, and there isn't even a :hover rule or something so bug 1192558 wouldn't even help... Maybe we should check for cursor: pointer in this case or such?

Checking for cursor: pointer is an interesting idea. Hadn't thought of that before. The way I was originally thinking to solve this class of problem was just to walk up the tree more, instead of stopping at the body here. We could walk up to the document element instead.

See Also: → 1633961

The cursor:pointer approach seems to work well, and fixes both this and bug 1633961. Since that's the more targeted approach (vs walking up the tree more) let's do that, and if we run into sites where it doesn't work we can revisit this.

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d3423c8b31af Treat elements with cursor:pointer CSS as event fluffing targets. r=emilio
Severity: -- → normal
Priority: -- → P2

https://hg.mozilla.org/mozilla-central/rev/d3423c8b31af

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox77: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

Comment on attachment 9144350 [details]
Bug 1632941 - Treat elements with cursor:pointer CSS as event fluffing targets. r?emilio

Beta/Release Uplift Approval Request

  • User impact if declined: Fenix users may find it hard to tap on certain elements, as their tap attempts get redirected to other nearby elements.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Per STR in the bug. See also STR in bug 1633961 which should also be fixed by this fix.
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): The patch refines a heuristic to figure out which elements are "clickable". It could be that the new condition includes too many things, in which case users may have the same problem (not being able to easily tap on desired elements) but on other websites or in other places. I think it's unlikely, but it's possible that the new set of "bad cases" is larger than the old set of "bad cases" so that's a risk.
  • String changes made/needed:
Attachment #9144350 - Flags: approval-mozilla-beta?
Flags: qe-verify+
No longer blocks: 1633961

Comment on attachment 9144350 [details]
Bug 1632941 - Treat elements with cursor:pointer CSS as event fluffing targets. r?emilio

Not a new regression in 76 and 76 is already into RC. Let's let this ride the 77 train.

Attachment #9144350 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
QA Whiteboard: [qa-triaged]

Verified as fixed on 77.0a1 - Nightly build from 5/5.
Devices: Samsung Galaxy Note 10 (Android 10), Nokia 6 (Android 7.1.1).

Status: RESOLVED → VERIFIED
status-firefox77: fixed → verified
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: