Focus events

[j3parker]

This is my first pass at #7981

Looking for some early feedback. I'm not sure that what I'm doing in commit_focus_transaction is 100% correct and I'm still trying to wrap my head around the spec (focusing/unfocusing/focus update steps.) I'm only doing focus/focusout at the moment while I try to understand what's going on.

When I combine this with what I have in #7757 things seem to work. What would be the best way of adding tests for something like this independent of that other issue?

[mbrubeck]

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

^{components/script/dom/document.rs, line 523 [r2] (raw file):}
This TODO comment should be updated.

---

Comments from the review on Reviewable.io

[j3parker]

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

^{components/script/dom/document.rs, line 523 [r2] (raw file):}
Got it - thanks!

---

Comments from the review on Reviewable.io

[Manishearth]

Reviewed 4 of 5 files at r1.
Review status: 3 of 5 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

^{components/script/dom/document.rs, line 989 [r3] (raw file):}
We should leave comments here with a FIXME that this is incomplete (doesn't handle related targets)

ditto to where its called

---

^{components/script/dom/document.rs, line 999 [r3] (raw file):}
spec says "to not bubble"

https://html.spec.whatwg.org/multipage/interaction.html#fire-a-focus-event

---

Comments from the review on Reviewable.io

[Manishearth]

Reviewed 1 of 5 files at r1, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

Comments from the review on Reviewable.io

[Manishearth]

Looks good so far.

I think web-platform-tests should already have tests for this, really. Try running ./mach test-wpt --log-raw tmp.log (this takes a long time, adding --processes 4 makes it faster), followed by ./mach update-wpt tmp.log will update test expectations.

[j3parker]

Review status: 4 of 5 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

^{components/script/dom/document.rs, line 989 [r3] (raw file):}
Done - I pre-emptively made it an argument on fire_focus_event.
It seems to me we'd have to be careful implementing this because of iframe security etc. (not wanting to pass a node from a different-origin iframe on a focus event up to the parent when focus leaves the iframe.) Does that sound right?

---

^{components/script/dom/document.rs, line 999 [r3] (raw file):}
D'oh, thanks

---

Comments from the review on Reviewable.io

[Manishearth]

Still needs a lot of the other things from the issue being implemented. Though I am fine with landing this as-is so we can work on it incrementally. (@jdm ?)

Also needs test expectation updates to land.

---

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

Comments from the review on Reviewable.io

[Manishearth]

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.

---

^{components/script/dom/document.rs, line 989 [r3] (raw file):}
Yes, that does. The spec should have a provision for that, though.

---

Comments from the review on Reviewable.io

[j3parker]

There were some more changes to WPT expectations that don't seem relevant that I split over here: j3parker@061c799

I haven't had time to triage them yet - hopefully tonight after work.

[Manishearth]

One of those failures is related, we should add FocusEvent to https://github.com/servo/servo/blob/master/tests/wpt/mozilla/tests/mozilla/interfaces.html#L72

[j3parker]

Thanks - I'll look through the rest tonight (EST)

[bors-servo]

☔ The latest upstream changes (presumably #7984) made this pull request unmergeable. Please resolve the merge conflicts.

[Manishearth]

Any updates?

[frewsxcv]

@j3parker have any ideas how close this is to completion? No problem if you're too busy, just let us know

[paulrouget]

@j3parker are you still working on this bug?

[jdm]

@paulrouget Seems safe to say no, given the multiple months with no reply after previous queries.

[frewsxcv]

@j3parker Thanks for the effort! Feel free to reopen if you want to want to work on it again and it's not already done!

[j3parker]

Oh wow I'm sorry!

I set up email filtering on Github (because we've been using it at my workplace more) and long story short these didn't get to my email and I forgot otherwise.

Happy to see it's been picked up though :) Cheers.