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

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
a month ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

({dev-doc-complete})

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
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)
(Assignee)

Comment 4

3 years ago
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
(Assignee)

Comment 6

3 years ago
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
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/67407d1829a38cbbd9ac20f5cac0e1a5c6c3d8f8
Bug 1270386 - Move unprefixed fullscreen event handlers from GlobalEventHandlers to Document. r=smaug
(Assignee)

Comment 10

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4a7e67e0fe50970d559eab440f0643799eb719d
Bug 1270386 - Move unprefixed fullscreen event handlers from GlobalEventHandlers to Document. r=smaug
(Assignee)

Comment 11

3 years ago
(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.

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e4a7e67e0fe5
Status: NEW → RESOLVED
Last Resolved: 3 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)
(Assignee)

Comment 14

3 years ago
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
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.