Expand addDocumentToFullScreenChangeEventQueue at the call sites for clarity

Expand addDocumentToFullScreenChangeEventQueue at the call sites for clarity

[philipj_slow, 2014-06-29 00:53:15]

PTAL. This is the more aggressive change mentioned in
https://codereview.chromium.org/354393002/

[philipj_slow, 2014-06-29 01:21:24]

https://codereview.chromium.org/354403002/diff/1/Source/core/dom/FullscreenEl...
File Source/core/dom/FullscreenElementStack.cpp (right):

https://codereview.chromium.org/354403002/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:312: enqueueChangeEvent(**i);
Bah, adding some asserts revealed that this change was not equivalent, although
apparently there's no test coverage for the case where an element becomes the
target instead of the document. (That's possible because even though the stack
is empty, m_fullScreenElement may be non-null.)

That makes this CL useless. The unprefixed events should always target document
and not bubble, so the code paths will have to be split in any case, but I was
hoping to actually understand the existing logic first. Neither case in this
function makes much sense to me, one element could get the fullscreenchange
event but if there were others on the stack they won't get an event, so scripts
can't count on the event anyway?

[falken, 2014-06-30 02:29:41]

https://codereview.chromium.org/354403002/diff/1/Source/core/dom/FullscreenEl...
File Source/core/dom/FullscreenElementStack.cpp (right):

https://codereview.chromium.org/354403002/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:312: enqueueChangeEvent(**i);
On 2014/06/29 01:21:24, philipj wrote:
> Bah, adding some asserts revealed that this change was not equivalent,
although
> apparently there's no test coverage for the case where an element becomes the
> target instead of the document. (That's possible because even though the stack
> is empty, m_fullScreenElement may be non-null.)
> 
> That makes this CL useless. The unprefixed events should always target
document
> and not bubble, so the code paths will have to be split in any case, but I was
> hoping to actually understand the existing logic first. Neither case in this
> function makes much sense to me, one element could get the fullscreenchange
> event but if there were others on the stack they won't get an event, so
scripts
> can't count on the event anyway?

I don't really understand this FullscreenElementStack stuff either. It's
entirely possible the existing code has some oversights like this.

[philipj_slow, 2014-06-30 21:48:11]

Closing this, maybe when I've poked more at this I'll understand it well enough
to actually clarify it instead of breaking it :)