[webkit-reviews] review granted: [Bug 201024] [Picture-in-Picture Web API] Implement HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture() : [Attachment 380945] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 15 06:25:56 PDT 2019
Eric Carlson <eric.carlson at apple.com> has granted Peng Liu
<peng.liu6 at apple.com>'s request for review:
Bug 201024: [Picture-in-Picture Web API] Implement
HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture()
https://bugs.webkit.org/show_bug.cgi?id=201024
Attachment 380945: Patch
https://bugs.webkit.org/attachment.cgi?id=380945&action=review
--- Comment #30 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 380945
--> https://bugs.webkit.org/attachment.cgi?id=380945
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=380945&action=review
> Source/WebCore/Modules/pictureinpicture/PictureInPictureWindow.cpp:59
> + return false;
This will prevent any page with a PictureInPictureWindow from entering the
PageCache. Can we do better?
> Source/WebCore/Modules/pictureinpicture/PictureInPictureWindow.h:47
> + int width() const { return m_width; }
> + int height() const { return m_height; }
You aren't maintaining PiP window 'state', which is important here as spec
says:
The [width/height] attribute MUST return the [width/height] in CSS pixels
of
the Picture-in-Picture window associated with pictureInPictureElement if
the state is opened. Otherwise, it MUST return 0.
> Source/WebCore/dom/Document.cpp:109
> +#if ENABLE(PICTURE_IN_PICTURE_API)
> +#include "HTMLVideoElement.h"
> +#endif
Nit: conditionally included headers are typically listed after the
unconditional ones.
More information about the webkit-reviews
mailing list