Open Bug 1523418 Opened 2 years ago Updated 2 years ago

unpredictable link-following behaviour when pointer-events set to 'none', seems to depend on image size

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P3)

66 Branch
All
Other
defect

Tracking

()

UNCONFIRMED

People

(Reporter: philipmorant, Unassigned)

Details

(Whiteboard: [specification][type:bug])

Attachments

(2 files)

What did you do?

1. js has mousedown handler which calls event.preventDefault(), also mouseup handler which calls event.preventDefault()
2. click on link

What happened?

firefox follows the link

What should have happened?

In firefox versions prior to 66 the same code prevented following links. I verified this by re-starting the same profile using the current release grade firefox (64).

I don't see any relevant changes listed on
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66
or
https://www.fxsitecompat.com/en-CA/versions/66/

Is this change intended ?

Is there anything else we should know?

Component: Events → Untriaged
Product: developer.mozilla.org → Firefox
Version: unspecified → 66 Branch
Summary: event.preventDefault() no longer stops firefox following links → in a mouseup handler event.preventDefault() no longer stops firefox following links

Please provide a testcase either as URL or as html attachment and please include complete steps to reproduce.

Flags: needinfo?(philipmorant)
Component: Untriaged → Event Handling
Product: Firefox → Core
Flags: needinfo?(philipmorant)
Summary: in a mouseup handler event.preventDefault() no longer stops firefox following links → unpredictable link-following behaviour when pointer-events set to 'none' in a mousemove handler

Load as a temporary addon in about:debugging.

Please ignore the 2 steps listed above in "What did you do?" (doesn't seem to be editable btw).

Version 66 has broken some event handling code which worked very reliably in earlier versions. That code was designed to listen for mouse drags and to scroll the page instead of drag n drop. Default actions are similarly prevented, via pointer-events: none. But in version 66 the pointer-events: none is frequently and unpredictably ignored.

To reproduce:

Install the attached xpi file.

https://www.theguardian.com/film/2019/feb/01/rewriting-the-past-do-historical-movies-have-to-be-accurate
Drag the mouse on the large image, the one with the actors including Remi Malek as Freddie Mercury. On my system (ubuntu) the link is followed every time. Prior to 66, never followed.

On a smaller image on https://en.wikipedia.org/wiki/Main_Page, the same mouse drag results in a link follow roughly one in seven attempts. Dragging left to right across the centre of an image however results in link follow roughly one time in two.

NB the mouse drag must start and finish on the image, not go outside.

Mouse drag on the span in
<a href='whatever.jpg'>
This is a textnode inside the anchor.
<span style='color: red;'>This is inside a span.</span>
This is a second textnode inside the anchor.
</a>
results in link follow every single time (but never, in versions prior to 66).

Comment on attachment 9040772 [details]
Webextensions content script with event handlers, for testing

Here's the code:

'use strict';

window.addEventListener('mousedown', event => {

const mousedownTarget = event.target;

event.view.document.addEventListener('dragstart', event => event.preventDefault(), console.log(`dragstart`), {once: true});

event.view.document.addEventListener('mousemove', event => {
    console.log(`mousemove; set ${mousedownTarget.nodeName}.pointerEvents to 'none'`);
    mousedownTarget.style.pointerEvents = 'none';
}, {once: true});

event.view.document.addEventListener('mouseup', event => mousedownTarget.style.pointerEvents = 'auto', {once: true});

}, false);

(In reply to philipmorant from comment #4)

Comment on attachment 9040772 [details]
Webextensions content script with event handlers, for testing

Here's the code:

'use strict';

window.addEventListener('mousedown', event => {

const mousedownTarget = event.target;

event.view.document.addEventListener('dragstart', event => event.preventDefault(), console.log(`dragstart`), {once: true});

event.view.document.addEventListener('mousemove', event => {
    console.log(`mousemove; set ${mousedownTarget.nodeName}.pointerEvents to 'none'`);
    mousedownTarget.style.pointerEvents = 'none';
}, {once: true});

event.view.document.addEventListener('mouseup', event => mousedownTarget.style.pointerEvents = 'auto', {once: true});

}, false);

Thanks for the test case! It helps find a regression window.

Regarding the event.preventDefault() on the dragstart handler:

If you comment the dragstart handler out then the problem can still be reproduced, but only with very short drag strokes. If firefox kicks off a DnD operation (once the stroke is long enough) then neither version 65 nor 66 will follow the link.

After further testing, it turns out that the problem is still present when pointer-events is set to 'none' in the mousedown handler, or even if set statically before the mouse events ever happen.

So, re-summarising, it looks like the behaviour of pointer-events has changed, the mouse events are a red herring, and there's unpredictable behaviour where the regression (if that's what it is) depends apparently on the size of the image.

Summary: unpredictable link-following behaviour when pointer-events set to 'none' in a mousemove handler → unpredictable link-following behaviour when pointer-events set to 'none', seems to depend on image size

In 66 you can get behaviour similar to version 65 if you also set the HTMLAnchorElement's pointerEvents to 'none'. So it looks like 66 changes pointerEvents so that it now allows event bubbling (possibly depending on whether preventDefault() is called). This would be a documentation bug, by which I mean that the release notes for 66 don't mention it as yet.

The inconsistent link following is still real though. It's as if someone had programmed

if (el instanceof HTMLImageElement && getComputedStyle(el).pointerEvents == 'none') {
    if (Math.random() * el.naturalWidth > 100) {
        bubble(event); // ... and the parent anchor follows the link
    }
}

Whether or not CSS pointer-events changes affects to the hit testing depends on when style/layout is flushed. Have we perhaps optimized out some flush? Anyhow, regression range is needed.

I had been noticing the pointer-events issue for a while before reporting this issue - maybe 2 weeks. But it's possible that the pointer-events change is entirely separate from whatever is causing the hit testing inconsistency.

Oh, I eventually got the regression window - https://hg.mozilla.org/integration/mozilla-inbound/json-pushes?changeset=61570c16c2d564c24fab36713fb169c4144453e9&full=1
2019-02-05T14:27:00: DEBUG : Found commit message:
Bug 1512259 - No need to special case <button> in marionette, r=ato

Olli might have ideas! :)

https://bugzilla.mozilla.org/show_bug.cgi?id=1089326 is more likely.
So click will be dispatched on the common ancestor of mousedown and mouseup now.
And if pointerEvents = 'none' is set on some descendant of <a> element, the mouseup will still go to the
<a>, and it will get a click.

Attached file testcase

This shows FF66 behaves similarly to Chrome. FF65 is different.

Marking invalid, but please reopen if I'm still missing something here.

Release notes will be updated https://bugzilla.mozilla.org/show_bug.cgi?id=1089326#c90

Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID

Further analysis shows that while the span-on-anchor may be fine, img-on-anchor has unexpected results when the mouse moves during a click.

Using the javascript in the xpi on the map <img> at URL
https://en.wikipedia.org/wiki/Breviary_of_Alaric

Test 1:
Click & drag the mouse horizontally across the centre of the image, taking care to remain within the rectangle where the underlying anchor inline element is. On that page impression that 66 gives me on a 1920x1080 desktop monitor @ 170% zoom (maybe depends on zoom and/or screen size, not sure) the anchor inline element falls inline with the header "Significance" on the LHS. I use Developer Tools to see that rectangle.

Result: firefox follows the link (anchor is a common ancestor)

Test 2:
As test 1, except one of mousedown or mouseup falls outwith the inline anchor element's rectangle.

Result: firefox does not follow the link (no common ancestor)

This may well be consistent with the common ancestor policy, but nevertheless end users like me may be surprised. I had assumed that the underlying anchor would be coextensive with its child elements.

Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Priority: -- → P3
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.