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

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+11]

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.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

It was initially added to HTMLElement in bug 807075, but I don't see reason there.

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: MozReview Request: Bug 1270386 - Move fullscreen and pointerlock event handlers from GlobalEventHandlers to Document. r?smaug

Review commit: https://reviewboard.mozilla.org/r/50737/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50737/

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

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.)

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

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?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

yes, feature detection is what I'm worried about.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

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/

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

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

Comment 9

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/589b2826c6c5 - see bug 1274104 comment 13

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

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

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

(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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4a7e67e0fe5

Comment 13

[Jean-Yves Perrier [:teoli]]

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?

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

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.

Comment 15

[Jean-Yves Perrier [:teoli]]

Created: 
https://developer.mozilla.org/en-US/docs/Web/API/Document/onfullscreenchange
https://developer.mozilla.org/en-US/docs/Web/API/Document/onfullscreenerror
Updated: 
https://developer.mozilla.org/en-US/Firefox/Releases/49#Others
https://developer.mozilla.org/en-US/docs/Web/API/Document
https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers