Poster set after playback begins should be ignored
Created attachment 398547 [details] Patch
Comment on attachment 398547 [details] Patch Nice, we got rid of setDisplayMode() entirely.
Created attachment 398756 [details] Patch for landing
<rdar://problem/62605114>
Created attachment 398782 [details] Patch for landing
Committed r261341: <https://trac.webkit.org/changeset/261341> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398782 [details].
(In reply to EWS from comment #6) > Committed r261341: <https://trac.webkit.org/changeset/261341> This seems to have made this test very flaky: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/intrinsic_sizes.htm Tracked in Bug 211612.
Reverted r261341 and r261392 for reason: Caused multiple regression test failures Committed r261409: <https://trac.webkit.org/changeset/261409>
Created attachment 399051 [details] Patch
Created attachment 399059 [details] Patch
Created attachment 399061 [details] Patch
Created attachment 399133 [details] Patch
Comment on attachment 399133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399133&action=review Is there code that stops the poster loading once we load the first frame? > Source/WebCore/ChangeLog:22 > +020-05-11 Antoine Quint <graouts@apple.com> Oops, deleted a "2" here. > Source/WebCore/html/HTMLVideoElement.cpp:136 > + if (hasAvailableVideoFrame()) > + return; Not really about this patch: I wish we had a more consistent rule about when to call through to the base class in parseAttribute functions. If it was up to me, all parseAttribute functions would call through to the base class always, even when they handle the attribute, except when they are trying to disable some specific work the base class does. But it’s more efficient to *not* call through to the base class if you know it has no work to do, so some might prefer our current status quo where we often don’t call through. > Source/WebCore/html/HTMLVideoElement.cpp:286 > + else if (displayMode() < Poster && !hasAvailableVideoFrame()) > setDisplayMode(Poster); Is the idea here that if hasAvailableVideoFrame() is true, the display mode will be changing to Video very soon? Why not change it to Video right now?
Comment on attachment 399133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399133&action=review >> Source/WebCore/ChangeLog:22 >> +020-05-11 Antoine Quint <graouts@apple.com> > > Oops, deleted a "2" here. Oops indeed, I wonder how I did that! >> Source/WebCore/html/HTMLVideoElement.cpp:286 >> setDisplayMode(Poster); > > Is the idea here that if hasAvailableVideoFrame() is true, the display mode will be changing to Video very soon? Why not change it to Video right now? That is the idea, but I like your suggestion - there is no reason to wait. I'll make that change, thanks!
Created attachment 399148 [details] Patch for landing
Committed r261576: <https://trac.webkit.org/changeset/261576> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399148 [details].