[webkit-reviews] review granted: [Bug 60142] Exiting full screen causes <video> element inline controls to become visible : [Attachment 93029] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 60142: Exiting full screen causes <video> element inline controls to become
visible
https://bugs.webkit.org/show_bug.cgi?id=60142

Attachment 93029: Patch
https://bugs.webkit.org/attachment.cgi?id=93029&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=93029&action=review

It seems like if you quickly enter and then exit fullscreen you might get two
fullscreenchange events, both after you are out of fullscreen. A little
strange.

Also, it seems this patch doesn’t apply, so you are not getting any EWS
testing.

> Source/WebCore/dom/Document.cpp:4921
> +    m_fullScreenChangeEventTargetQueue.append(m_fullScreenElement);
>      m_fullScreenElement = 0;

It should be more efficient to call m_fullScreenElement.release() here and then
you don’t need to set m_fullScreenElement to 0 on the next line.

> Source/WebCore/dom/Document.cpp:4992
> +    m_fullScreenChangeEventTargetQueue.append(m_fullScreenElement);

It should be more efficient to call m_fullScreenElement.release() here.