Closed Bug 1270386 Opened 4 years ago Closed 4 years ago

Move event handlers of fullscreen and pointerlock events from GlobalEventHandlers to Document

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

Details

(Keywords: dev-doc-complete, Whiteboard: btpp-active)

Attachments

(1 file)

We only dispatch fullscreen and pointerlock events on Document. Event handler on elements would never get the event they want. So apparently those event handler should be in Document rather than in GlobalEventHandlers.
It was initially added to HTMLElement in bug 807075, but I don't see reason there.
Comment on attachment 8749058 [details]
MozReview Request: Bug 1270386 - Move fullscreen and pointerlock event handlers from GlobalEventHandlers to Document. r?smaug

https://reviewboard.mozilla.org/r/50737/#review47445

So other browsers seem to have at least on*fullscreenchange in Elements. Why we think this change is safe?

(hmm, and now I don't know how to use mozreview for this... I think it just forces me to clear review request even though I'm just asking for from comment here.)
Attachment #8749058 - Flags: review?(bugs)
https://reviewboard.mozilla.org/r/50737/#review47445

The spec doesn't say so... and it seems Edge doesn't have onfullscreen* on Elements, but both Blink and Edge have onwebkitfullscreen* on Elements...

I guess we should at least try to move the unprefixed ones to Document. I have no idea how could this move break things if they are never invoked? Could people use that for feature detection?
yes, feature detection is what I'm worried about.
Whiteboard: btpp-active
Comment on attachment 8749058 [details]
MozReview Request: Bug 1270386 - Move fullscreen and pointerlock event handlers from GlobalEventHandlers to Document. r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50737/diff/1-2/
Attachment #8749058 - Flags: review?(bugs)
Attachment #8749058 - Flags: review?(bugs) → review+
Comment on attachment 8749058 [details]
MozReview Request: Bug 1270386 - Move fullscreen and pointerlock event handlers from GlobalEventHandlers to Document. r?smaug

https://reviewboard.mozilla.org/r/50737/#review50678
https://hg.mozilla.org/integration/mozilla-inbound/rev/67407d1829a38cbbd9ac20f5cac0e1a5c6c3d8f8
Bug 1270386 - Move unprefixed fullscreen event handlers from GlobalEventHandlers to Document. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4a7e67e0fe50970d559eab440f0643799eb719d
Bug 1270386 - Move unprefixed fullscreen event handlers from GlobalEventHandlers to Document. r=smaug
(In reply to Phil Ringnalda (:philor) from comment #9)
> Backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/589b2826c6c5 - see
> bug 1274104 comment 13

That shouldn't be related to this bug. It could be bug 931445 which was landed in the same push. Reland the patch here.
https://hg.mozilla.org/mozilla-central/rev/e4a7e67e0fe5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Xidorn: quick question wrt docs. It looks to me that this bugs only moved unprefixed fullscreen-related event handlers and has no influence on pointerlock-related event handlers: prefixed ones are still on GEH, and unprefixed ones are not implemented (but will be on Document, in another bug).

Is this correct?
Flags: needinfo?(bugzilla)
Yes, exactly. The bug summary is outdated. smaug worried about that moving the prefixed ones could potentially affect feature detection somewhere, so prefixed ones are left untouched.
Flags: needinfo?(bugzilla)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.