Setting src asynchronously of calling play() on a media with preload none doesn't play |
|||
Issue descriptionTest page: https://mounirlamouri.github.io/sandbox/media/set-src-timeout-no-preload.html STR: - load test page Expected result: video plays Actual result: video does not play Additional information: - works on Firefox. - the controls are in a broken state.
,
Oct 21 2016
,
May 12 2017
Hi, I just reproduced this issue. As per the W3C spec, the moment play() is called the autoplay option (if it exists is set to false). However, I am not sure if it is correct to expect that setting src on a video object which has had play() called on it before , should actually play the changed src. I can investigated further and try to isolate a fix , if the behavior as outlined in the test case is actually valid.
,
May 30 2017
Updating this after testing on browserstack: - Edge (15): works (like Firefox) - Safari (10.1): does not work (like Chrome) Most likely, this is a WebKit bug that stayed in Blink.
,
May 30 2017
Actually, when I say "a bug", I meant a different behaviour. As comment #3 mentioned, it's unclear whether Chrome/Safari or Firefox/Edge are correct here.
,
May 30 2017
Hi, Thanks for getting back to me and clarifying. I have added a short patch which gets the behavior as seen in Firefox here: https://codereview.chromium.org/2910683003/ Do let me know your thoughts. Thanks, George.
,
May 30 2017
I couldn't find a clear answer from the specification. However, in Blink, this seems to happen: - playback starts with 'play' and 'waiting' events firing - setIgnorePreloadNone is called - src attribute is parsed - ignore_preload_none_ is reversed back to false - 'loadstart' event is fired - load algorithm is running - load does not happen because preload is set to 'none' - loading is suspended and 'suspend' event is fired What might be interesting to understand is why `paused_` is never set back to true.
,
May 30 2017
Right before the src attribute is set, paused is false and networkState is 0. Correctly per https://html.spec.whatwg.org/multipage/embedded-content.html#media-element-load-algorithm, paused remains false afterwards. I don't know of anything in the spec that would explain why it doesn't play, so this looks like a bug in Blink/WebKit. The way that ignore_preload_none_ is maintained is suspect, and the TODO before HTMLMediaElement::InvokeLoadAlgorithm() is relevant. I think that for all cases where ignore_preload_none_ is reset before a InvokeLoadAlgorithm() call, it should actually only be set back to false if InvokeLoadAlgorithm() will set paused_ to true.
,
May 30 2017
There is a spec problem here, though, namely that ignore_preload_none_ isn't explained at all. https://www.w3.org/Bugs/Public/show_bug.cgi?id=28921 is about that for load(). Spec PRs welcome :)
,
May 30 2017
This is where https://codereview.chromium.org/1522463003 this refactoring was done. Before this change load algorithm and selection algorithm are split across prepareForLoad(), loadInternal() and prepareToPlay() and ignore_preload_none_(formerly known as m_havePreparedToPlay) is being handled in prepareForLoad() and prepareToPlay(). And this refactoring (https://codereview.chromium.org/1522463003) to match spec was done partially with a TODO to take care of ignore_preload_none_ in later CLs. And my attempt to implement resource selection algorithm per spec in https://codereview.chromium.org/1810513002 was suspended due to dependency on loader.
,
May 30 2017
Regarding the "dependency on loader", is that only about cancelable tasks, or is it a larger problem?
,
May 30 2017
It is only about cancelable tasks (the time at which microtask is fired is creating problem for media document related test cases). This is the problematic code https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Document.cpp?q=document.cpp+package:%5Echromium$&l=5489 There is a FIXME for that piece of code (https://bugs.chromium.org/p/chromium/issues/detail?id=425790) but that doesn't look like being fixed anytime soon.
,
May 30 2017
Thanks for the feedback. Do let me know if I should continue working on the issue and I can fix the issue further. Thanks, George.
,
May 31 2017
If this is affecting real sites, then fixing it with a workaround would be reasonable. Perhaps it would work to change HTMLMediaElement::LoadResource so that we never defer the load if not paused?
,
Jun 5 2017
Hello foolip and mlamouri, I have updated the patch and added a test. All the tests should now pass. Please do review and let me know your thoughts. Thanks.
,
Jul 4 2017
Hello foolip and mlamouri, Would you be able to review the patch anytime : https://codereview.chromium.org/2910683003/ Thanks, George.
,
Feb 6 2018
Hello Mounir, I have uploaded the new set of changes here: https://chromium-review.googlesource.com/c/chromium/src/+/867920 Thanks, George. |
|||
►
Sign in to add a comment |
|||
Comment 1 by mlamouri@chromium.org
, Aug 9 2016