1272411 - [Context Menu] Provide a 1.25x faster video speed option in the video controls. Also add UI to loop playback of media files

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

(From Bug 1126282 comment #0)
> The current video speed options don't work very well for spoken audio. 1.5x
> is too fast. I'd like a video option in between. Currently, I end up using
> the devtools to set this.

> YouTube offers 1.25x for their HTML5 player. Would 1.25x work for you? It
> would be nice to be consistent with them.

Comment 1

[Horatiu Lazu]

Hi, I would be interested in doing this for my first bug. Would it be possible? Thanks.

Comment 2

[Philip Chee]

(In reply to Horatiu Lazu from comment #1)
> Hi, I would be interested in doing this for my first bug. Would it be
> possible? Thanks.

Yes certainly. We won't change the status to ASSIGNED until we see a patch from you. So please proceed. If you need any assistance you can find us on IRC  irc://moznet/seamonkey

Comment 3

[Richa]

Is the bug assigned to somebody? can i work on it?

Comment 4

[Horatiu Lazu]

I'm no longer working on it, I'm working on another bug right now

Comment 5

[Sherman Wang]

If nobody else is currently working this, I'd like to tackle it as my first bug.

Comment 6

[Sherman Wang]

Attachment: 1272411.patch

port of Firefox implementation for 1.25x video speed option and video looping UI

Comment 7

[Sherman Wang]

Seeing as Richa hasn't been active since mid-October, I've uploaded a patch that ports the implementation from Firefox including their updated verbiage for video speeds.  A few other minor fixes from the Firefox block of code came along for the ride as well; please advise if they should be removed.

Comment 8

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8822113 [details] [diff] [review]
1272411.patch

Looks good to me. Will check it out later.

Comment 9

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8822113 [details] [diff] [review]
1272411.patch

1.25 and loop work fine for me from the context menu.

Comment 10

[Philip Chee]

Comment on attachment 8822113 [details] [diff] [review]
1272411.patch

Nice patch!

> -    this.showItem("context-video-fullscreen", this.onVideo);
> +    this.showItem("context-video-fullscreen", this.onVideo &&
> +                  this.target.ownerDocument.fullscreenElement == null);
This can be written as:
   this.showItem("context-video-fullscreen", this.onVideo &&
                 !this.target.ownerDocument.fullscreenElement);

> +      this.setItemAttr("context-media-playbackrate-050", "disabled", hasError);
> +      this.setItemAttr("context-media-playbackrate-100", "disabled", hasError);
> +      this.setItemAttr("context-media-playbackrate-125", "disabled", hasError);
> +      this.setItemAttr("context-media-playbackrate-150", "disabled", hasError);
> +      this.setItemAttr("context-media-playbackrate-200", "disabled", hasError);
These are not needed as the whole popup is disabled.

> -<!ENTITY mediaPlaybackRate.label        "Playback Speed">
> -<!ENTITY mediaPlaybackRate.accesskey    "b">
> -<!ENTITY mediaPlaybackRate050.label     "Slow Motion (½×)">
> -<!ENTITY mediaPlaybackRate100.label     "Normal Speed">
> -<!ENTITY mediaPlaybackRate150.label     "High Speed (1½×)">
> -<!ENTITY mediaPlaybackRate150.accesskey "H">
> -<!ENTITY mediaPlaybackRate200.label     "Double Speed">
> -<!ENTITY mediaPlaybackRate200.accesskey "D">
Don't change these please.

> +<!ENTITY mediaPlaybackRate125.label     "Fast (1.25×)">
> +<!ENTITY mediaPlaybackRate125.accesskey "F">
Please use UTF 8 character for ¼ e.g. "Fast Speed (1¼×)"

> +<!ENTITY mediaLoop.label              "Loop">
> +<!ENTITY mediaLoop.accesskey          "L">
L is taken by "Copy Video _L_ocation".
I think "o" is still available.

Comment 11

[Sherman Wang]

Attachment: 1272411-2.patch

Updated patch per feedback from Comment #10

Comment 12

[Frank-Rainer Grahl (:frg)]

Sherman,

you need to set r? or it will get lost in time. Philip and IanN are the reviewers for SeaMonkey. I can do it for you like the last time but then I get the direct notifications too.

Comment 13

[Sherman Wang]

(In reply to Frank-Rainer Grahl from comment #12)
> Sherman,
> 
> you need to set r? or it will get lost in time. Philip and IanN are the
> reviewers for SeaMonkey. I can do it for you like the last time but then I
> get the direct notifications too.

Thanks for catching that.  I think I have the r? flag set correctly now.

Comment 14

[Ian Neal]

Comment on attachment 8823078 [details] [diff] [review]
1272411-2.patch

r/a=me

Comment 15

[Frank-Rainer Grahl (:frg)]

Sherman, I pushed the bug for you. The next time please add a commit message to it. 

https://hg.mozilla.org/comm-central/rev/52def7491f3399fe04989958a4508989daee4571