Closed Bug 463155 Opened 11 years ago Closed 11 years ago
Add video/audio controls to message context menu, since the regular controls depend on Java
The video tag works fine in trunk mailnews. This includes the "autoplay" attribute. STR: Paste this code into an email or news post: <video src="http://www.double.co.nz/video_test/ascannerdarkly480.ogg" autoplay="true"></video> Open the composition The video plays (a good thing) No controls to stop the video (a bad thing) This is because the controls were implemented in XBL ?? Bug 461136 ported the Firefox context menu to SeaMonkey, maybe that's an answer to the lack of controls in TB
Component: General → Mail Window Front End
OS: Windows XP → All
QA Contact: general → front-end
Hardware: PC → All
Dup of bug 449358?
Only up until the last sentence of comment 0, which given the conclusion of bug 449358 for 1.9.1, is a blocker.
Assignee: nobody → philringnalda
No longer blocks: 453943
Status: NEW → ASSIGNED
No longer depends on: 453928
Target Milestone: --- → Thunderbird 3.0b3
I think the minimum solution we need is some way to disable the autoplay of this element, providing a context menu still requires that someone find the menu and use it.. For added fun I tried out the audio version and you get the same effect, except there is no visible object to interact with. <audio src="http://www.double.co.nz/video_test/ascannerdarkly480.ogg" autoplay="true"></audio> Stopping the autoplay functionality of both these elements would solve the problem of possibly receiving messages with video or audio playback that can't be stopped.
I had several paragraphs typed about how disabling autoplay (which is bug 449531, with zero traction, or something better scoped that isn't yet filed) without having a context menu is actually worse than just disabling audio/video, since you then have no way of playing it at all, just the tease of "it's supported, it just doesn't ever play," but then what you said about no visible object finally clicked: with autoplay, that means ten million griefers mass-mailing <audio src="imlookingatgayporn.ogg" autoplay>, without autoplay it means practically speaking we don't support audio. Lightly edited, from a conversation with bz: <bz>: You could do an evil hack, like have some chrome that detects <video>s in the mail and injects controls and listens for events using a listener in chrome to detect clicks on those controls and scripts the <video> elements accordingly. <philor>: you're reminding me of my only experience with that sort of thing, where the feed preview page put a little chrome in content, and moz_bug_r should have made several hundred dollars off all the bugs he's filed on it <bz>: it injected a chrome js object into content, I'm just suggesting injecting DOM nodes and not having any script running in content (which was the other difference with the preview page) and not injecting _chrome_ dom nodes. You create the DOM nodes using createElement on the content document. You then insert them in the content DOM. I think this would be a _lot_ safer than what feeds did. The only script running is your chrome script. It only listens for trusted mouse events (so ones that actually correspond to user clicks), that sort of thing. It might be a bit annoying to implement, but it should be doable.... • bz hates giving advice like this <bz>: I feel like I'm suggesting that you do things in the most backwards way possible. Unfortunately, the non-backwards ways are broken, some by design <philor>: sadly, I don't know who we can assign it to, I only know who we can assign "+MOZ_MEDIA=" to So, this bug needs an owner and a resummary, but I don't have any idea who that owner is. I can keep it and use it as the bug where we stop building MOZ_MEDIA, but that's all I can promise to have time for.
Status: ASSIGNED → NEW
Whiteboard: [needs owner]
As a workaround, we should very much do +pref("media.autoplay.enabled", false) somewhere in the prefs when bug 478982 lands.
bug 478892 landed on mozilla-central, but not yet in 1.9.1 No reason not to add the override pref to mail/app/profile/all-thunderbird.js now. The only thing that bothers me is, messages/newsgroup posts will still show a media window, with no way to start the media, and leave the user totally clueless. Although I am loathe to say this "A partially supported feature can be worse that no feature at all" pref("media.ogg.enabled", false); pref("media.wave.enabled", false); While the above "might" eliminate all appearance of the media, my guess would be that we would simply ignore that message content completely. Which is the case where you have plugins disabled, and the embed tag in a message points to a remote source. (You never know that content is there) At the very least, we should inform the user that some content was not rendered If not a popup, then at least an entry in the error console.
(In reply to comment #7) > bug 478892 landed on mozilla-central, but not yet in 1.9.1 > > No reason not to add the override pref to mail/app/profile/all-thunderbird.js > now. I assume you meant bug 478982. I wonder how seamonkey want to tackle that pref...
Yes, bug 478982 (thx magnus) has now landed in 1.9.1, so some testing: Setting media.autoplay.enabled to false does in fact stop the playing. The bad news is, that it does not block the media frame. So you get the "remote content frame" regardless of your setting of mailnews.message_display.disable_remote_image (that could be another bug, as I thought that pref should block all remote content, image and other content as well) If I set media.ogg.enabled to "false", the video tag is completely ignored. But I think we owe it to the user to let them know when content is ignored because of a hidden pref setting. (error console notification at least) Maybe there is an application here for the Activity Manager. Like "Video blocked due to prefs settings"
(In reply to comment #9) > The bad news is, that it does not block the media frame. > So you get the "remote content frame" regardless of your setting of > mailnews.message_display.disable_remote_image > (that could be another bug, as I thought that pref should block all remote > content, image and other content as well) The fact that it attempts to fetch the content if the src attribute is remote is definitely a separate bug. If you could spin that off and CC me on it, that would be much appreciated. > If I set media.ogg.enabled to "false", the video tag is completely ignored. > > But I think we owe it to the user to let them know when content is ignored > because of a hidden pref setting. Some sort of notification does seem in order. We could conceivably display a dummy media frame with informational text inside and perhaps a link to override the default of not displaying the remote contents. I'll be interested to hear Bryan's thoughts on this.
(In reply to comment #10) > > But I think we owe it to the user to let them know when content is ignored > > because of a hidden pref setting. > > Some sort of notification does seem in order. We could conceivably display a > dummy media frame with informational text inside and perhaps a link to override > the default of not displaying the remote contents. I'll be interested to hear > Bryan's thoughts on this. I think this just depends on what we want to do and can do for the person. There is a striking similarity between this and the remote images system. I don't think we'd actually want to offer for people to turn on remote video completely as much as we'd want to allow people to turn it on - just now or always for this person. Not sure if those aspects are possible.
(In reply to comment #11) > > I think this just depends on what we want to do and can do for the person. > There is a striking similarity between this and the remote images system. I > don't think we'd actually want to offer for people to turn on remote video > completely as much as we'd want to allow people to turn it on - just now or > always for this person. Not sure if those aspects are possible. I think we have exactly the same scenario here as in the case of remote images. The fixes there (address book and universal pref) should be used here as well. In the mean time, since the pref "media.ogg.enabled" is set to true by default, that means that any mail with the tag can be confirmed as a "valid address" in the same way as a remote image, whether autoplay is enabled or not. (Although, I haven't heard of any instances of this in the wild.) We should probably have a spinoff bug to address the possibilities. "Review content policy for Video/Audio tags" I think it's important for TB to stay current for HTML5 and stay on the leading edge.
Filed bug 480674 for a content policy
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b4
I've pinged Justin but not heard back, I think dmose may have as well. So stealing this bug as we need to get a patch up for review before the string freeze - I should have a patch ready in a few hours.
Assignee: dolske → bugzilla
Whiteboard: [has l10n impact] → [has l10n impact][patch almost ready]
Adds Play/Pause and Mute/Unmute controls. When you right-click a video element, the normal context menu will be replaced by play/pause and mute/unmute options depending on the state of the video. We could potentially add Copy Location and Save As items as well, but I don't see that as blocking and they could probably be added via an extension later if necessary. I'll file follow-up bugs for those. I'm still examining the audio element I'm not quite sure what we can do there yet.
Comment on attachment 399091 [details] [diff] [review] Add Play/Pause, Mute/Unmute controls this looks good for the video element.
Attachment #399091 - Flags: ui-review?(clarkbw) → ui-review+
(In reply to comment #15) > I'm still examining the audio element I'm not quite sure what we can do there > yet. Should we be packaging some default <audio> graphic that we can make the target for right clicking? I'm not sure if that's possible.
(In reply to comment #18) > (In reply to comment #15) > > I'm still examining the audio element I'm not quite sure what we can do there > > yet. > > Should we be packaging some default <audio> graphic that we can make the target > for right clicking? I'm not sure if that's possible. Or even a text character ♫ looks appropriate
Whiteboard: [has l10n impact][patch almost ready] → [has l10n impact][needs review philor]
Comment on attachment 399111 [details] [diff] [review] Add Play/Pause, Mute/Unmute controls v2 >diff --git a/mail/locales/en-US/chrome/messenger/messenger.dtd b/mail/locales/en-US/chrome/messenger/messenger.dtd >--- a/mail/locales/en-US/chrome/messenger/messenger.dtd >+++ b/mail/locales/en-US/chrome/messenger/messenger.dtd >@@ -684,6 +684,16 @@ you can use these alternative items. Oth > <!ENTITY contextPrintPreview.label "Print Preview"> > <!ENTITY contextPrintPreview.accesskey "v"> > >+<!-- Media (video/audio) controls --> >+<!ENTITY contextPlay.label "Play"> >+<!ENTITY contextPlay.accesskey "P"> >+<!ENTITY contextPause.label "Pause"> >+<!ENTITY contextPause.accesskey "P"> >+<!ENTITY contextMute.label "Mute"> >+<!ENTITY contextMute.accesskey "M"> >+<!ENTITY contextUnmute.label "Unmute"> >+<!ENTITY contextUnmute.accesskey "m"> Does using the same accesskeys for Play/Pause and Mute/Unmute really work? Is only one part of the combination ever active?
(In reply to comment #21) > Does using the same accesskeys for Play/Pause and Mute/Unmute really work? Is > only one part of the combination ever active? If you look in the initMediaPlayerItems function, you'll see these are exclusive items. There isn't any point in putting up Mute and Unmute at the same time anyway (I suppose you could disable an option, but that's not necessary). Also (before anyone asks ;-) ) when right-clicking on a video these items are the only ones available in the context menu which is why we can also use access keys that are used elsewhere in the menu.
Comment on attachment 399111 [details] [diff] [review] Add Play/Pause, Mute/Unmute controls v2 >+ // Several mutall exclusive items.... play/pause, mute/unmute, show/hide Maybe people will be more likely to realize why we can use the same accesskeys if you choose a word other than "mutall" ;)
Attachment #399111 - Flags: review?(philringnalda) → review+
Checked in with comment fixed: http://hg.mozilla.org/comm-central/rev/51ff72b37ec3
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [has l10n impact][needs review philor] → [has l10n impact]
I've added a note to devmo TB 3 page that we've now "enabled" this element: https://developer.mozilla.org/En/Thunderbird_3_for_developers
(In reply to comment #22) > (In reply to comment #21) > > Does using the same accesskeys for Play/Pause and Mute/Unmute really work? Is > > only one part of the combination ever active? > > If you look in the initMediaPlayerItems function, you'll see these are > exclusive items. There isn't any point in putting up Mute and Unmute at the > same time anyway (I suppose you could disable an option, but that's not > necessary). It's still a bad idea to use the same accesskey for two strings - not for en-US but for localizations. The translation of "mute" and "unmute" to a foreign language might end up with strings that don't share any character!
So it's a good thing Mark isn't stupid, and didn't do that, isn't it?
Oh, right, I should better read patches and not just comments ;-)
You need to log in before you can comment on or make changes to this bug.