478982 - Gecko users may need to prevent autoplay for video/audio content in messages

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[David Ascher (:davida)]

Thunderbird 3 will be built on top of Gecko 1.9.1., with JavaScript disabled in messages, as a security measure.  Currently video/audio content that has autoplay=true will start on display, with no way for the user to stop it.  That implies pretty awful spam scenarios.

We'd like to be able to tell gecko to never autoplay.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We should fix this, but you could also work around it by handling the onloadeddata event in a chrome event listener, removing the 'autoplay' attribute there, and calling pause() on the element for good measure.

Comment 2

[Nochum Sossonko [:Natch]]

Attachment: Something like this?

I think this should do it, I also changed the video controls so that they show if the video has the autoplay attr. but hasn't started.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 362855 [details] [diff] [review]
Something like this?

Awesome thanks! Dolske'd better check the videocontrols changes.

Comment 4

[Justin Dolske [:Dolske]]

Comment on attachment 362855 [details] [diff] [review]
Something like this?

>-            if (!video.autoplay || !this.Utils.dynamicControls) {
>+            if (!(video.autoplay && video.paused) || !this.Utils.dynamicControls) {

I don't think this will work. :(

The spec (4.8.10.6) says that .paused should be true -- even for autoplay videos -- until the ready state hits HAVE_ENOUGH_DATA and playback begins. I think you're assuming .paused is initially false (and stays false, barring explicit action) for autoplay videos.

I coincidentally just ran into this same situation with bug 470983... I want to show a throbber while loading, and have it go away when |canplay| fires (for not-autoplay) or |playing| fires (for autoplay). But if the user agent ignores autoplay, or if something calls pause() on the video before playback begins, there's no way to know that |playing| will never fire (and so the throbber would run forever).

One idea would be to have the controls look at the browser prefs, and change this check to !(video.autoplay && prefs.autoplayEnabled). I think this would be useful for other reasons too. But it's tricky, because the controls are unprivileged. Maybe the native video code could help by setting an attribute on the anonymous content.

Or the spec could be changed/extended... Force .autoplay to false, and ignore changes? Add a readonly .willAutoplay flag? Something else?

Comment 5

[Nochum Sossonko [:Natch]]

(In reply to comment #4)
> (From update of attachment 362855 [details] [diff] [review])
> >-            if (!video.autoplay || !this.Utils.dynamicControls) {
> >+            if (!(video.autoplay && video.paused) || !this.Utils.dynamicControls) {
> 
> I don't think this will work. :(

I'd be surprised if this didn't work as the video code relies on paused being false in order to autoplay, although maybe I'm wrong  :).

See http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLMediaElement.cpp#1185 for example (which is where I put the check for the pref and where autoplay is acted upon).

fwiw, I did test this and it worked fine, but if you want me to go try hacking around, I'll definitely give it a try.

Comment 6

[Nochum Sossonko [:Natch]]

> false in order to autoplay, although maybe I'm wrong  :).

err, s/false/true. AFAICT paused is set to true from the start, see http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLMediaElement.cpp?mark=554-554,563-563#553

Comment 7

[Nochum Sossonko [:Natch]]

Ok, so I'm planning on adding a bool readonly willAutoplay attribute to the video element, is that ok?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think perhaps mozAutoplayEnabled.

Comment 9

[Nochum Sossonko [:Natch]]

Attachment: Patch ver. 2

Ok, so I did the mozAutoplayEnabled boolean attribute and now the video controls check for that in addition to the check for autoplay. Note, this pref is useless in a JS environment as the author would simply call play() on the video element and can easily simulate auto playing with that. I suppose that's not a problem though.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, that's not a problem.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  // Readonly boolean indicating whether or not the video will autoplay.
+  // This can occur if 1) there is no autoplay attr. or 2) the user has set
+  // the media.autoplay.enabled pref to false.

This comment is incorrect. You wrote the right code; mAutoplayEnabled has nothing to do with the autoplay attribute. It only checks whether the element would autoplay if the autoplay attribute was set.

Comment 12

[Nochum Sossonko [:Natch]]

Right that comment is bogus. Before I post a new patch, do I have to bump the uuid on nsIDOMHTMLMediaElement?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes.

Comment 14

[Nochum Sossonko [:Natch]]

Attachment: fix the comment and rev the uuid [Checkin: Comment 15]

Comment 15

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/2d8105e38927

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 363243 [details] [diff] [review]
fix the comment and rev the uuid
[Checkin: Comment 15]

Does not apply to 1.9.1:
{
patching file content/html/content/src/nsHTMLMediaElement.cpp
Hunk #1 succeeded at 166 with fuzz 2 (offset -83 lines).
Hunk #2 FAILED at 621
Hunk #3 FAILED at 722
Hunk #4 FAILED at 1293
3 out of 4 hunks FAILED
}

Comment 17

[Nochum Sossonko [:Natch]]

(In reply to comment #16)
Bug 462455 needs to land first.

Comment 18

[Biju]

(In reply to comment #1)
> We should fix this, but you could also work around it by handling the
> onloadeddata event

So are we downloading video/audio content in the mail messages?
Dont we stop it like images?

 
On 2/25/09 8:02 PM, Robert O'Callahan wrote:
>  
> That's a good point. We really shouldn't do any downloading by default.

--- On Thu, 26/2/09, David Ascher <[e-mail]> wrote:

> From: David Ascher <[e-mail]>
> Why isn't happening on a bug? ;-)
> 

I am no longer a TB user so I wanted somebody to test it for me.
Hence put message at
http://groups.google.com/group/mozilla.dev.quality/browse_thread/thread/249ec66996e5895d#

Comment 19

[Biju]

Also we need to ask Yahoo Mail, GMail, Hotmail etc whether they sanitize it properly to protect users privacy.

Comment 20

[Joe Sabash [:JoeS1]]

(In reply to comment #19)
> Also we need to ask Yahoo Mail, GMail, Hotmail etc whether they sanitize it
> properly to protect users privacy.

I don't think WE need to ask them anything. You should, if you use those clients

Gmail renders only the tags that it permits, with no option to control that decision.
Tb and SeaMonkey do offer that decision in the view option.
View>>Message body as..>>Simple html will only render the tags that you pref.

That's a big difference in UX

Gecko users have a choice Gmail users do not.
If you want someone else to make that decision for you , then use Gmail or some other webmail. If you think you should make that choice, use TB or Seamonkey mail.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6d7caf9aa203