Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gecko Bug 1931692] Make MouseEvent::DuplicatePrivateData() keep storing `mPresContext #49822

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moz-wptsync-bot
Copy link
Collaborator

Currently, offsetX and offsetY are defined by CSSOM spec [1]. The spec
defines that when the event is not being dispatched, these values should be
same as pageX and pageY. However, we started computing the values with
the latest data since bug 1203404 for compatibility with the other browsers.
However, the test does not check the trusted event behavior. When the event
is a trusted event, Event::DuplicatePrivateData() clears
Event::mPresContext. Then, calling Event::GetOffsetCoords with specifying
mPresContext aborts to compute it [2]. Therefore, our mouse events return
0 for offsetX and offsetY after propagation.

Blink and WebKit caches offsetX and offsetY values when they are referred
first time [3][4]. Therefore, their values depend on when they are
referred. I don't think we should follow their tricky behavior immediately.
However, at least, we should return same computed result until the layout and
scroll position are not changed. Therefore, this patch makes MouseEvent
keeps storing mPresContext even after calling
UIEvent::DuplicatePrivateData() and use it when calling
Event::GetOffsetCoords.

Finally, this adds a WPT under uievents to check current behavior in the
simplest case (i.e., neither scroll position nor layout is not changed after
starting the propagation).

  1. https://drafts.csswg.org/cssom-view/#extensions-to-the-mouseevent-interface
  2. https://searchfox.org/mozilla-central/rev/2493c256dbff4e3c7e51a7fc61115df887b87e9e/dom/events/Event.cpp#707-708
  3. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/events/mouse_event.cc;l=518;drc=0aa8c88324c762d0907de203da053c4545c4fd6b
  4. https://searchfox.org/wubkat/rev/d46c14cf9008c4385cf0ac7919561b278d047e85/Source/WebCore/dom/MouseRelatedEvent.cpp#220

Differential Revision: https://phabricator.services.mozilla.com/D232589

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1931692
gecko-commit: 42de37e6c1b427a7a0b66aeaa2084dd4a1d8a408
gecko-reviewers: smaug

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Firefox project.

Currently, `offsetX` and `offsetY` are defined by CSSOM spec [1].  The spec
defines that when the event is not being dispatched, these values should be
same as `pageX` and `pageY`.  However, we started computing the values with
the latest data since bug 1203404 for compatibility with the other browsers.
However, the test does not check the trusted event behavior.  When the event
is a trusted event, `Event::DuplicatePrivateData()` clears
`Event::mPresContext`.  Then, calling `Event::GetOffsetCoords` with specifying
`mPresContext` aborts to compute it [2].  Therefore, our mouse events return
`0` for `offsetX` and `offsetY` after propagation.

Blink and WebKit caches `offsetX` and `offsetY` values when they are referred
first time [3][4].  Therefore, their values depend on **when** they are
referred.  I don't think we should follow their tricky behavior immediately.
However, at least, we should return same computed result until the layout and
scroll position are not changed.  Therefore, this patch makes `MouseEvent`
keeps storing `mPresContext` even after calling
`UIEvent::DuplicatePrivateData()` and use it when calling
`Event::GetOffsetCoords`.

Finally, this adds a WPT under `uievents` to check current behavior in the
simplest case (i.e., neither scroll position nor layout is not changed after
starting the propagation).

1. https://drafts.csswg.org/cssom-view/#extensions-to-the-mouseevent-interface
2. https://searchfox.org/mozilla-central/rev/2493c256dbff4e3c7e51a7fc61115df887b87e9e/dom/events/Event.cpp#707-708
3. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/events/mouse_event.cc;l=518;drc=0aa8c88324c762d0907de203da053c4545c4fd6b
4. https://searchfox.org/wubkat/rev/d46c14cf9008c4385cf0ac7919561b278d047e85/Source/WebCore/dom/MouseRelatedEvent.cpp#220

Differential Revision: https://phabricator.services.mozilla.com/D232589

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1931692
gecko-commit: cfb7d95a6f12643d327dd20e324c1747219d718b
gecko-reviewers: smaug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants