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

RESOLVED FIXED in mozilla1.9.2a1

Status

()

defect
--
major
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: davida, Assigned: Natch)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tb3needs])

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

11 years ago
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.
Flags: blocking1.9.1?
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.
Flags: blocking1.9.1? → blocking1.9.1+
Assignee

Comment 2

11 years ago
Posted patch Something like this? (obsolete) — Splinter Review
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.
Attachment #362855 - Flags: superreview?(roc)
Attachment #362855 - Flags: review?(roc)
Whiteboard: [tb3needs]
Attachment #362855 - Flags: superreview?(roc)
Attachment #362855 - Flags: superreview+
Attachment #362855 - Flags: review?(roc)
Attachment #362855 - Flags: review?(dolske)
Attachment #362855 - Flags: review+
Comment on attachment 362855 [details] [diff] [review]
Something like this?

Awesome thanks! Dolske'd better check the videocontrols changes.
Whiteboard: [tb3needs] → [tb3needs][needs review]
Assignee

Updated

11 years ago
Assignee: nobody → highmind63
Status: NEW → ASSIGNED
Attachment #362855 - Flags: review?(dolske) → review-
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?
Assignee

Comment 5

11 years ago
(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.
Assignee

Comment 6

11 years ago
> 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
Assignee

Comment 7

11 years ago
Ok, so I'm planning on adding a bool readonly willAutoplay attribute to the video element, is that ok?
I think perhaps mozAutoplayEnabled.
Assignee

Comment 9

11 years ago
Posted patch Patch ver. 2 (obsolete) — Splinter Review
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.
Attachment #362855 - Attachment is obsolete: true
Attachment #363065 - Flags: superreview?(roc)
Attachment #363065 - Flags: review?(roc)
Blocks: 463155
Assignee

Updated

11 years ago
Attachment #363065 - Flags: review?(dolske)
+  // 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.
Attachment #363065 - Flags: review?(dolske) → review+
Assignee

Comment 12

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

Comment 14

11 years ago
Attachment #363065 - Attachment is obsolete: true
Attachment #363243 - Flags: superreview?(roc)
Attachment #363243 - Flags: review?(roc)
Attachment #363065 - Flags: superreview?(roc)
Attachment #363065 - Flags: review?(roc)
Attachment #363243 - Flags: superreview?(roc)
Attachment #363243 - Flags: superreview+
Attachment #363243 - Flags: review?(roc)
Attachment #363243 - Flags: review+
Keywords: checkin-needed
Whiteboard: [tb3needs][needs review] → [tb3needs][needs landing]
http://hg.mozilla.org/mozilla-central/rev/2d8105e38927
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [tb3needs][needs landing] → [tb3needs][needs 191 landing]
Target Milestone: --- → mozilla1.9.2a1
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
}
Attachment #363243 - Attachment description: fix the comment and rev the uuid → fix the comment and rev the uuid [Checkin: Comment 15]
Keywords: checkin-needed
Version: unspecified → Trunk
Assignee

Comment 17

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

Comment 18

11 years ago
(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 <david.ascher@gmail.com> wrote:

> From: David Ascher <david.ascher@gmail.com>
> 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

11 years ago
Also we need to ask Yahoo Mail, GMail, Hotmail etc whether they sanitize it properly to protect users privacy.
(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.

Updated

10 years ago
Blocks: 493217
You need to log in before you can comment on or make changes to this bug.