Closed Bug 1332956 Opened 3 years ago Closed 3 years ago

Video's audio autoplays without playing the video

Categories

(Core :: DOM: Core & HTML, defect)

50 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 + wontfix
firefox53 + wontfix
firefox54 --- fixed

People

(Reporter: jdm, Assigned: bzbarsky)

References

()

Details

(Keywords: regression)

Attachments

(4 files, 3 obsolete files)

When I load this page, the audio from the video element starts playing as soon as it's loaded. The video does not give any indication that it's playing, and pressing play will start a second stream of the audio as the video actually starts to play. The only way to stop the original audio stream is the mute the tab.
It did not happen in FF 47, and does happen in FF 52.
mozregression shows that this was introduced in FF 50 so far.
I can see this problem on the latest nightly on my MAC. 
Alastor, 
Can you help check this problem?
Component: Audio/Video → Audio/Video: Playback
Flags: needinfo?(alwu)
I cannot explain it, but the problem is clearly reproducible starting with https://hg.mozilla.org/mozilla-central/rev/b3e20967ccde and I cannot reproduce it using https://hg.mozilla.org/mozilla-central/rev/fcf14af43c5a.
Blocks: 1288390
Flags: needinfo?(nfroyd)
Assignee: nobody → alwu
No longer blocks: 1288390
Flags: needinfo?(alwu)
regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=815c7d8dfebe361f7d93c1a5efd86ab0894e9577&tochange=fc189ba3703da6ae63a9fd39977c889e1970a486

regressed by:
fc189ba3703d	Jean-Yves Avenard — Bug 1285928: [mp4] Ignore non-supported entries in edit list. r=kentuckyfriedtakahe
Blocks: 1285928
Flags: needinfo?(jyavenard)
The root cause is the <object> would automatically play any video by creating a new video document.
Flags: needinfo?(jyavenard)
The <object> would automatically play the video (mov) behind the <video>, it was totally overlapped by the <video> so that we can't see its control interface and we can't stop it.

Note, I guess this issue has existed for VERY VERY long time, because it have existed before jya's changing which was mentioned in comment5. Before jya's changing, the <object> still play the video, but the video is silent so that we can't aware about it.

---

After some testing, automatically playing video seems the feature of the <object>, so there comes up another question, what is the correct behavior when the <object> was put inside the hierarchy of <video>?

It seems that the website wants to use <object> as <source>, but it's totally different.

Following is their code,
> <video controls>
>   <source src=".../chrisjones-full.ogv" type="video/ogg">
>   <source src=".../chrisjones-full.mov" type="video/quicktime">
>   <object type="video/quicktime" data=".../chrisjones-full.mov" autoplay="false"></object>
> </video>

---

Hi, Bz,
Could you give us some suggestion? 
What's the correct behavior of <object> in this kind of situation?
Thanks!
Flags: needinfo?(bzbarsky)
Version: unspecified → 50 Branch
(In reply to Josh Matthews [:jdm] from comment #4)
> I cannot explain it, but the problem is clearly reproducible starting with
> https://hg.mozilla.org/mozilla-central/rev/b3e20967ccde and I cannot
> reproduce it using https://hg.mozilla.org/mozilla-central/rev/fcf14af43c5a.

It sounds like we have a better explanation for this than clang warning flags being disabled (whew!).
Flags: needinfo?(nfroyd)
Tracking recent regression for 52/53
> What's the correct behavior of <object> in this kind of situation?

To not play the video.  The HTML spec is not easily linkable here, but if you load https://html.spec.whatwg.org/multipage/embedded-content.html#the-object-element and then search for "run the following steps to (re)determine what the object element represents" step 2 says:

  If the element has an ancestor media element .... then jump to the step below labeled fallback.

It looks like we implement this for <embed> (as of bug 1262184) but not <object>.  We should probably hoist this up to be checked for both...
Flags: needinfo?(bzbarsky) → needinfo?(kyle)
Attachment #8829758 - Flags: review?(bzbarsky)
Comment on attachment 8829758 [details]
Bug 1332956 - stop loading the source if parent node is media element.

https://reviewboard.mozilla.org/r/106766/#review107912

As far as I can tell, this doesn't actually implement what the spec says, does it?

What you probably want to do is hoist up the code we have for <embed> right now and reuse it, assuming that code matches the spec for <object> too.

And of course you should have a test.
Attachment #8829758 - Flags: review?(bzbarsky) → review-
In particular, see testing/web-platform/tests/html/semantics/embedded-content/the-embed-element/embed-ignored-in-media-element.html and dom/base/test/test_bug1263696.html, though it really would be better if the latter were a wpt test too.
Flags: needinfo?(kyle)
After reading the spec mentioned in comment10, the solution should be the part of the algorithm "queued or actively running must delay the load event of the element's node document".

Since I don't really familiar with this spec, I'll free this bug and let someone who has better understanding about this spec to finish it.
Assignee: alwu → nobody
Component: Audio/Video: Playback → DOM
> the solution should be the part of the algorithm "queued or actively running must delay the load event of the element's node document"

No, that has nothing to do with it.  Comment 13 says what should happen.  But OK.  So much for my attempt to not have to do this myself...  I'll trade you reviews on stylo, I guess.
Assignee: nobody → bzbarsky
I can grab this if you're busy.
Attachment #8834299 - Attachment is obsolete: true
Attachment #8834299 - Flags: review?(kyle)
Attachment #8834301 - Attachment is obsolete: true
Attachment #8834301 - Flags: review?(kyle)
Attachment #8834419 - Attachment is obsolete: true
Attachment #8834419 - Flags: review?(kyle)
Attachment #8834310 - Flags: review?(kyle) → review+
Attachment #8834300 - Flags: review?(kyle) → review+
Attachment #8834503 - Flags: review?(kyle) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/116291cf00b5
part 1.  Move our existing mochitest for embed-inside-object over to web-platform-tests.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cb85309542e
part 2.  Refactor embed-ignored-in-media-element to make it simple to test the same thing with <object>.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdea29a8b0f3
part 3.  Implement the same behavior for <object>-inside-<object> and <object>-inside-mediaelement as we do for <embed> already.  r=qdot
sorry Boris, had to back this out for merge conflicts like:
warning: conflicts while merging dom/html/HTMLSharedObjectElement.h! (edit, then use 'hg resolve --mark')


seems this is from https://bugzilla.mozilla.org/show_bug.cgi?id=1334330 that landed before on central
Flags: needinfo?(bzbarsky)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/565a4eba23d4
Backed out changeset bdea29a8b0f3 
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd9462ab5c72
Backed out changeset 9cb85309542e 
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e4db804e21c
Backed out changeset 116291cf00b5 for causing merge conflicts with mozilla-central
<sigh>.  It wasn't even a real merge conflict.  It was a difference in the _context_ and a trivial one: someone had renamed the argument to the function before the one I was changing.  :(
Flags: needinfo?(bzbarsky)
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47b7e3dd8e56
part 1.  Move our existing mochitest for embed-inside-object over to web-platform-tests.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7d277e859d
part 2.  Refactor embed-ignored-in-media-element to make it simple to test the same thing with <object>.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d9e64633666
part 3.  Implement the same behavior for <object>-inside-<object> and <object>-inside-mediaelement as we do for <embed> already.  r=qdot
Given that I got tracking emails about this: I'm not very comfortable backporting this.  It's a significant behavior change with serious risk of site breakage.
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #31)
> Given that I got tracking emails about this: I'm not very comfortable
> backporting this.  It's a significant behavior change with serious risk of
> site breakage.

Fair enough.  Marking wontfix for branches.
Blocks: 1247356
Since this fix has automated coverage, I don't think manual testing would be of much use here.

Boris, if you think Manual QA should instead be looking at this, feel free to flip the qe-verify flag or ni? me directly.
Flags: qe-verify-
Depends on: 1383478
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.