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

Fire mouseenter and mouseleave events #27413

Merged
merged 5 commits into from Jul 31, 2020

Conversation

utsavoza
Copy link
Contributor

@utsavoza utsavoza commented Jul 27, 2020

The PR primarily consists of changes for:

  • Fixing the order in which mousemove events are fired.
  • Firing mouseenter and mouseleave events.

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/document.rs
  • @KiChjang: components/script/dom/document.rs, components/script_traits/lib.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 27, 2020
@highfive
Copy link

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

@utsavoza utsavoza marked this pull request as ready for review July 27, 2020 17:32
@jdm
Copy link
Member

jdm commented Jul 27, 2020

r? @nox

@highfive highfive assigned nox and unassigned SimonSapin Jul 27, 2020
@nox
Copy link
Contributor

nox commented Jul 30, 2020

The code looks correct but this will probably need changes to the test expectations.

@bors-servo try=wpt

bors-servo added a commit that referenced this pull request Jul 30, 2020
Fire mouseenter and mouseleave events

The PR primarily consists of changes for:
- Fixing the order in which `mousemove` events are fired.
- Firing `mouseenter` and `mouseleave` events.
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26958
- [ ] There are tests for these changes
@bors-servo
Copy link
Contributor

⌛ Trying commit 962e620 with merge f1c835a...

@bors-servo
Copy link
Contributor

☀️ Test successful - status-taskcluster
State: approved= try=True

@nox
Copy link
Contributor

nox commented Jul 30, 2020

Aha, we didn't enable pointerevents tests. You need to add this to tests/wpt/include.ini:

[pointerevents]
  skip: false

You can add it after this stuff to keep things alphabetical:

[permissions]
  skip: false

@jdm
Copy link
Member

jdm commented Jul 30, 2020

I think pointerevents is unrelated to mouse events?

@utsavoza
Copy link
Contributor Author

utsavoza commented Jul 30, 2020

I have included some manual tests I'd used to verify mouse events' order.

@nox
Copy link
Contributor

nox commented Jul 31, 2020

I've checked locally and everything looks good now AFAICT.

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit de20715 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 31, 2020
@bors-servo
Copy link
Contributor

⌛ Testing commit de20715 with merge 0021a82...

@bors-servo
Copy link
Contributor

☀️ Test successful - status-taskcluster
Approved by: nox
Pushing 0021a82 to master...

@bors-servo bors-servo merged commit 0021a82 into servo:master Jul 31, 2020
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fire mouseenter/mouseleave events
6 participants