[webkit-reviews] review denied: [Bug 69063] [Chromium] Forward fullscreenchanged event to plugins : [Attachment 110026] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 6 23:54:00 PDT 2011


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied
polina at chromium.org's request for review:
Bug 69063: [Chromium] Forward fullscreenchanged event to plugins
https://bugs.webkit.org/show_bug.cgi?id=69063

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110026&action=review


> Source/WebKit/chromium/ChangeLog:63
> +>>>>>>> .r96838

oops

> Source/WebKit/chromium/public/WebPlugin.h:86
> +    virtual void handleFullscreenChangeEvent() { }

nit: didChangeFullscreenState would probably be more consistent with the
way we name other notifications of state changes.

> Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:194
> +	   m_webPlugin->handleFullscreenChangeEvent();

i'm still not sure that this is the best solution.  can the page intercept this

event and prevent its delivery to the WebPluginContainerImpl?  if it did that,
then perhaps we might miss a notification that we switched into fullscreen
mode?
could a page construct a fullscreenchange event and dispatch it manually to the

plugin to confuse the plugin?

this is why i was thinking that we should use the ChromeClient methods as a
signal since those cannot be intercepted or faked.


More information about the webkit-reviews mailing list