1474149 - Allow media seek amount so be configured

Status: NEW

(Toolkit :: Video/Audio Controls, enhancement, P5)

Description

[h-h]

When you play audio or video in Firefox, you can jump by a fixed amount of 15 seconds, as this help page explains: https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly#w_media-shortcuts.

Here are media files for testing. You may have to click on the pause/play button, before the keystrokes will be recognized:
- Audio: http://wilwheaton.typepad.com/files/wil_wheaton_vs_text_2_speech.mp3
- Video: http://www.belleslettres.eu/video/konjunktiv-irrealis.mp4

15 seconds is an unusual big seek step. These apps all have 5 seconds as their default seek step:
- YouTube
- foobar2000 (popular audio player for Windows)
- MPC-BE (video player for Windows)

So, a user should be able to modify the seek step length.

Comment 1

[h-h]

Attachment: Bug 1474149 - Allow media seek amount so be configured

When you play audio or video in Firefox, you can jump by a fixed amount of 15 seconds, as this help page explains: https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly#w_media-shortcuts.

Here are media files for testing. You may have to click on the pause/play button, before the keystrokes will be recognized:
- Audio: http://wilwheaton.typepad.com/files/wil_wheaton_vs_text_2_speech.mp3
- Video: http://www.belleslettres.eu/video/konjunktiv-irrealis.mp4

15 seconds is an unusual big seek step. These apps all have 5 seconds as their default seek step:
- YouTube
- foobar2000 (popular audio player for Windows)
- MPC-BE (video player for Windows)

So, a user should be able to modify the seek step length.

To accomplish that, this patch introduces the config option `media.seekStepLength`.

I changed `HTMLMediaElement`, because, in `videocontrols.xml`, unlike in other XBL files,
- `Services` (for `Services.prefs.getIntPref()`),
- `Cc` (for `Cc["@mozilla.org/preferences-service;1"]`) or
- `Components`/`Components.classes` (for `Components.classes["@mozilla.org/preferences-service;1"]`)
...were not available.

In accordance with the existing `HTMLMediaElement` API, the newly introduced property `mozSeekStepLength` has seconds as its unit. The `media.seekStepLength` config option is in milliseconds, because floats are converted from strings and milliseconds are already a popular unit in Firefox's preferences.

Should I add `, 15000` like in the following code, in case the config option was deleted?
```c++
double
HTMLMediaElement::MozSeekStepLength() const
{
  return Preferences::GetInt("media.seekStepLength", 15000) /*ms*/ / 1000.0;
}
```

Review commit: https://reviewboard.mozilla.org/r/255728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255728/

Comment 2

[Code Review Bot [:reviewbot]]

Comment on attachment 8990649 [details]
Bug 1474149 - Allow media seek amount so be configured

https://reviewboard.mozilla.org/r/255728/#review262404


Code analysis found 13 defects in this patch:
 - 13 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/content/widgets/videocontrols.xml:1489
(Diff revision 1)
>          if (String.fromCharCode(event.charCode) == " ") {
>            keystroke += "space";
>          }
>  
>          this.log("Got keystroke: " + keystroke);
> -        var oldval, newval;
> +        let oldval, newval;

Error: 'newval' is defined but never used. [eslint: no-unused-vars]

::: toolkit/content/widgets/videocontrols.xml:1517
(Diff revision 1)
>                this.video.muted = true;
>                break;
>              case "accel-upArrow": /* Unmute */
>                this.video.muted = false;
>                break;
> -            case "leftArrow": /* Seek back 15 seconds */
> +            case "leftArrow":          /* Seek backward...             */

Error: Multiple spaces found before '/* Seek backwa...*/'. [eslint: no-multi-spaces]

::: toolkit/content/widgets/videocontrols.xml:1518
(Diff revision 1)
>                break;
>              case "accel-upArrow": /* Unmute */
>                this.video.muted = false;
>                break;
> -            case "leftArrow": /* Seek back 15 seconds */
> -            case "accel-leftArrow": /* Seek back 10% */
> +            case "leftArrow":          /* Seek backward...             */
> +            case "rightArrow":         /* ...or forward a fixed amount */

Error: Multiple spaces found before '/* ...or forwa...*/'. [eslint: no-multi-spaces]

::: toolkit/content/widgets/videocontrols.xml:1519
(Diff revision 1)
>              case "accel-upArrow": /* Unmute */
>                this.video.muted = false;
>                break;
> -            case "leftArrow": /* Seek back 15 seconds */
> -            case "accel-leftArrow": /* Seek back 10% */
> -              oldval = this.video.currentTime;
> +            case "leftArrow":          /* Seek backward...             */
> +            case "rightArrow":         /* ...or forward a fixed amount */
> +            case "accel-leftArrow":    /* Seek backward...             */

Error: Multiple spaces found before '/* Seek backwa...*/'. [eslint: no-multi-spaces]

::: toolkit/content/widgets/videocontrols.xml:1521
(Diff revision 1)
>                break;
> -            case "leftArrow": /* Seek back 15 seconds */
> -            case "accel-leftArrow": /* Seek back 10% */
> -              oldval = this.video.currentTime;
> -              if (keystroke == "leftArrow") {
> -                newval = oldval - 15;
> +            case "leftArrow":          /* Seek backward...             */
> +            case "rightArrow":         /* ...or forward a fixed amount */
> +            case "accel-leftArrow":    /* Seek backward...             */
> +            case "accel-rightArrow": { /* ...or forward 10%            */
> +              let maxTime = this.video.duration /*s*/ ||

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1522
(Diff revision 1)
> -            case "leftArrow": /* Seek back 15 seconds */
> -            case "accel-leftArrow": /* Seek back 10% */
> -              oldval = this.video.currentTime;
> -              if (keystroke == "leftArrow") {
> -                newval = oldval - 15;
> -              } else {
> +            case "leftArrow":          /* Seek backward...             */
> +            case "rightArrow":         /* ...or forward a fixed amount */
> +            case "accel-leftArrow":    /* Seek backward...             */
> +            case "accel-rightArrow": { /* ...or forward 10%            */
> +              let maxTime = this.video.duration /*s*/ ||
> +                            this.maxCurrentTimeSeen /*ms*/ / 1000;

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1525
(Diff revision 1)
> -              if (keystroke == "leftArrow") {
> -                newval = oldval - 15;
> -              } else {
> -                newval = oldval - (this.video.duration || this.maxCurrentTimeSeen / 1000) / 10;
> -              }
> -              this.video.currentTime = (newval >= 0 ? newval : 0);
> +            case "accel-rightArrow": { /* ...or forward 10%            */
> +              let maxTime = this.video.duration /*s*/ ||
> +                            this.maxCurrentTimeSeen /*ms*/ / 1000;
> +              let seekStepLength = keystroke.startsWith("accel-") ?
> +                  maxTime * 0.10
> +                : this.video.mozSeekStepLength /*s*/;

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1527
(Diff revision 1)
> -              } else {
> -                newval = oldval - (this.video.duration || this.maxCurrentTimeSeen / 1000) / 10;
> -              }
> -              this.video.currentTime = (newval >= 0 ? newval : 0);
> -              break;
> -            case "rightArrow": /* Seek forward 15 seconds */
> +                            this.maxCurrentTimeSeen /*ms*/ / 1000;
> +              let seekStepLength = keystroke.startsWith("accel-") ?
> +                  maxTime * 0.10
> +                : this.video.mozSeekStepLength /*s*/;
> +              this.video.currentTime = keystroke.endsWith("leftArrow") ?
> +                  Math.max(this.video.currentTime - seekStepLength, 0)       // <-

Error: Multiple spaces found before '// <-'. [eslint: no-multi-spaces]

::: toolkit/content/widgets/videocontrols.xml:1528
(Diff revision 1)
> -                newval = oldval - (this.video.duration || this.maxCurrentTimeSeen / 1000) / 10;
> -              }
> -              this.video.currentTime = (newval >= 0 ? newval : 0);
> -              break;
> -            case "rightArrow": /* Seek forward 15 seconds */
> -            case "accel-rightArrow": /* Seek forward 10% */
> +              let seekStepLength = keystroke.startsWith("accel-") ?
> +                  maxTime * 0.10
> +                : this.video.mozSeekStepLength /*s*/;
> +              this.video.currentTime = keystroke.endsWith("leftArrow") ?
> +                  Math.max(this.video.currentTime - seekStepLength, 0)       // <-
> +                : Math.min(this.video.currentTime + seekStepLength, maxTime) // ->

Error: Missing semicolon. [eslint: semi]

::: toolkit/content/widgets/videocontrols.xml:1535
(Diff revision 1)
> +            }
>              case "home": /* Seek to beginning */
>                this.video.currentTime = 0;
>                break;
>              case "end": /* Seek to end */
> -              if (this.video.currentTime != this.video.duration) {
> +              if (this.video.currentTime /*s*/ != this.video.duration /*s*/) {

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1535
(Diff revision 1)
> +            }
>              case "home": /* Seek to beginning */
>                this.video.currentTime = 0;
>                break;
>              case "end": /* Seek to end */
> -              if (this.video.currentTime != this.video.duration) {
> +              if (this.video.currentTime /*s*/ != this.video.duration /*s*/) {

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1536
(Diff revision 1)
>              case "home": /* Seek to beginning */
>                this.video.currentTime = 0;
>                break;
>              case "end": /* Seek to end */
> -              if (this.video.currentTime != this.video.duration) {
> -                this.video.currentTime = (this.video.duration || this.maxCurrentTimeSeen / 1000);
> +              if (this.video.currentTime /*s*/ != this.video.duration /*s*/) {
> +                this.video.currentTime = this.video.duration /*s*/ ||

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

::: toolkit/content/widgets/videocontrols.xml:1537
(Diff revision 1)
>                this.video.currentTime = 0;
>                break;
>              case "end": /* Seek to end */
> -              if (this.video.currentTime != this.video.duration) {
> -                this.video.currentTime = (this.video.duration || this.maxCurrentTimeSeen / 1000);
> +              if (this.video.currentTime /*s*/ != this.video.duration /*s*/) {
> +                this.video.currentTime = this.video.duration /*s*/ ||
> +                                         this.maxCurrentTimeSeen /*ms*/ / 1000;

Error: Expected space or tab after '/*' in comment. [eslint: spaced-comment]

Comment 3

[h-h]

Comment on attachment 8990649 [details]
Bug 1474149 - Allow media seek amount so be configured

The patch concerns these triages:
- Toolkit -> Video/Audio Controls (reviewer Gijs, suggested)
- Core -> DOM: Core & HTML (reviewer Andrew Overhault, triage owner)

Comment 4

[h-h]

Attachment: Bug 1474149 - Allow media seek amount so be configured

Satisfied ESLint.

Review commit: https://reviewboard.mozilla.org/r/255730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255730/

Comment 5

[:Gijs (he/him)]

Comment on attachment 8990649 [details]
Bug 1474149 - Allow media seek amount so be configured

https://reviewboard.mozilla.org/r/255728/#review262470

I did a code review, but I haven't seen any real discussion about this change. (Please let me know if there was discussion somewhere else not linked from the bug!) I'd like other people to weigh in as to whether they think we should make it. Adding yet more hidden prefs is very rarely the best solution to (perceived) UX issues.

To provide some balance here, the builtin video controls in Chrome seem to have half-broken seek support. I couldn't get any seeking to happen on the wil wheaton mp3 file. On the videos on https://www.quirksmode.org/html5/tests/video.html , I could only get stepping to move in less-than-1s steps . On the `conjunctive irrealis` file, stepping proceeded in what looked like approx. 22-23 second steps. I have no idea why, perhaps it's to do with key frames in the source video, or it uses a 0.5%-of-total-length-of-video step, or something. Safari provides 15 second steps, and even provides buttons for this on either side of the play/pause button.

If we do anything (as opposed to nothing, ie wontfixing this bug) I'd be more inclined to just change these to use 5 second steps outright and not have the pref. If people want bigger steps they can either hold down the key or use the accel-arrow variants. Given how easy those alternatives are, I think that'd be a better change than the (significant) code impact in this patch.


If we do decide to go ahead and add a pref, we should probably have an automated test as well.

::: commit-message-ffb7b:1
(Diff revision 1)
> +Bug 1474149 - Allow media seek amount so be configured

Nit: s/so/to/

::: dom/webidl/HTMLMediaElement.webidl:127
(Diff revision 1)
>    // NB: for internal use with the video controls:
> -  [Func="IsChromeOrXBL"] attribute boolean mozAllowCasting;
> -  [Func="IsChromeOrXBL"] attribute boolean mozIsCasting;
> +  [Func="IsChromeOrXBL"]
> +  attribute boolean mozAllowCasting;
> +  [Func="IsChromeOrXBL"]
> +  attribute boolean mozIsCasting;
> +  readonly attribute double mozSeekStepLength;

You added a web-exposed property. I don't think that's right, I expect you'd want the same [Func] annotation as the other chrome/xbl-only properties. You may need to update DOM/webidl tests to deal with the added property.

::: toolkit/content/widgets/videocontrols.xml:1489
(Diff revision 1)
>          if (String.fromCharCode(event.charCode) == " ") {
>            keystroke += "space";
>          }
>  
>          this.log("Got keystroke: " + keystroke);
> -        var oldval, newval;
> +        let oldval, newval;

Instead of getting rid of both, can you just get rid of `newval` here and leave the up/down code for volume adjustment the same, instead of fetching `video.volume` more than once?

::: toolkit/content/widgets/videocontrols.xml:1517
(Diff revision 1)
> -            case "leftArrow": /* Seek back 15 seconds */
> -            case "accel-leftArrow": /* Seek back 10% */
> +            case "leftArrow":          /* Seek backward...             */
> +            case "rightArrow":         /* ...or forward a fixed amount */

Please use hg amend / histedit to collapse the two csets.

::: toolkit/content/widgets/videocontrols.xml:1527
(Diff revision 1)
> -            case "rightArrow": /* Seek forward 15 seconds */
> -            case "accel-rightArrow": /* Seek forward 10% */
> +                  Math.max(this.video.currentTime - seekStepLength, 0)       // <-
> +                : Math.min(this.video.currentTime + seekStepLength, maxTime) // ->

I don't think the <- / -> add anything here.

::: toolkit/content/widgets/videocontrols.xml:1537
(Diff revision 1)
>                this.video.currentTime = 0;
>                break;
>              case "end": /* Seek to end */
> -              if (this.video.currentTime != this.video.duration) {
> -                this.video.currentTime = (this.video.duration || this.maxCurrentTimeSeen / 1000);
> +              if (this.video.currentTime /*s*/ != this.video.duration /*s*/) {
> +                this.video.currentTime = this.video.duration /*s*/ ||
> +                                         this.maxCurrentTimeSeen /*ms*/ / 1000;

Instead of all the s/ms comments, maybe just rename the `maxCurrentTimeSeen` property to be `maxCurrentTimeSeenMs`. We can't rename the other things because they're web standards, and they're all in seconds anyway, so at least that'll clarify what's going on without having to mix in mini-comments everywhere.

Comment 6

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #5)
> Comment on attachment 8990649 [details]
> Bug 1474149 - Allow media seek amount so be configured
> 
> https://reviewboard.mozilla.org/r/255728/#review262470
> 
> I did a code review, but I haven't seen any real discussion about this
> change. (Please let me know if there was discussion somewhere else not
> linked from the bug!) I'd like other people to weigh in as to whether they
> think we should make it. Adding yet more hidden prefs is very rarely the
> best solution to (perceived) UX issues.

Jared, Chris, thoughts? Do we have a UX person who takes point on these types of decisions?

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #6)
> (In reply to :Gijs (he/him) from comment #5)
> > Comment on attachment 8990649 [details]
> > Bug 1474149 - Allow media seek amount so be configured
> > 
> > https://reviewboard.mozilla.org/r/255728/#review262470
> > 
> > I did a code review, but I haven't seen any real discussion about this
> > change. (Please let me know if there was discussion somewhere else not
> > linked from the bug!) I'd like other people to weigh in as to whether they
> > think we should make it. Adding yet more hidden prefs is very rarely the
> > best solution to (perceived) UX issues.
> 
> Jared, Chris, thoughts? Do we have a UX person who takes point on these
> types of decisions?

+chris-for-real-this-time. Sigh.

Comment 8

[Andrew Overholt [:overholt]]

I can't review this. I'll leave it up to cpearce to decide who should.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gijs (he/him) from comment #5)
> If we do anything (as opposed to nothing, ie wontfixing this bug) I'd be
> more inclined to just change these to use 5 second steps outright and not
> have the pref. If people want bigger steps they can either hold down the key
> or use the accel-arrow variants. Given how easy those alternatives are, I
> think that'd be a better change than the (significant) code impact in this
> patch.

I agree with this too. A hidden pref won't solve this for people as they won't know how to find it. I'm fine with a 5s step and longer steps with accel-arrow variants.

Comment 10

[h-h]

(In reply to :Gijs (he/him) from comment #5)
> I haven't seen any real discussion about this
> change. (Please let me know if there was discussion somewhere else not
> linked from the bug!) I'd like other people to weigh in as to whether they
> think we should make it.

Here's my attempt of a poll and more information: https://www.reddit.com/r/firefox/comments/8xiqh7/poll_playing_audiovideo_files_in_the_browser/.

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

I have handed the module ownership of media playback to Jean-Yves Avenard, so he should be the one to handle this.

Comment 12

[Jean-Yves Avenard [:jya]]

I have no particular preference between 15s or 5s... Having said that, most of the popular players I've used on various platforms use 10s for front/back:
MythTV (30s for forward, but 10s for backward which IMHO is best combination)
Netflix player on iPad and web player
Apple TV default player
Orange Livebox
VLC (default is 10s, but with shift-Left/Right it's 3s)

YouTube has both 10s and 5s seeking shortcuts.

So I'd say 10s is better, as jumping by 5s at a time, even with a repeat key would make quick travel in a video particularly slow.
I think 15s forward and 5s forward would be ideal: you can find the scene you want quickly, and if you went too far, just have more control going backward...

Now about the patch proposed, I see no reason why HTMLMediaElement would need to be modified nor the webidl interface, so that's r- for me. It can all be done in the video control layer, in plain JS

Comment 13

[Jean-Yves Avenard [:jya]]

(In reply to h-h from comment #10)

> Here's my attempt of a poll and more information:
> https://www.reddit.com/r/firefox/comments/8xiqh7/
> poll_playing_audiovideo_files_in_the_browser/.

You will get very biased answers just with how you formulated your question...

Could be summarise with: 15s sucks, but 5s is much better. Which one would you prefer? :)

Comment 14

[:Gijs (he/him)]

(In reply to Jean-Yves Avenard [:jya] from comment #12)
> I have no particular preference between 15s or 5s... Having said that, most
> of the popular players I've used on various platforms use 10s for front/back:
> MythTV (30s for forward, but 10s for backward which IMHO is best combination)
> Netflix player on iPad and web player
> Apple TV default player
> Orange Livebox
> VLC (default is 10s, but with shift-Left/Right it's 3s)
> 
> YouTube has both 10s and 5s seeking shortcuts.
> 
> So I'd say 10s is better, as jumping by 5s at a time, even with a repeat key
> would make quick travel in a video particularly slow.
> I think 15s forward and 5s forward would be ideal: you can find the scene
> you want quickly, and if you went too far, just have more control going
> backward...

We do already support longer jumps with accel-arrowkey...

> Now about the patch proposed, I see no reason why HTMLMediaElement would
> need to be modified nor the webidl interface, so that's r- for me. It can
> all be done in the video control layer, in plain JS

The commit message explains why this was done - there's no pref access from the videocontrols XBL code (it's mostly-unprivileged), or message managers or anything else. We don't have good alternative infrastructure to expose user preferences. Do you have a concrete alternative suggestion?

Comment 15

[h-h]

(In reply to Jean-Yves Avenard [:jya] from comment #13)
> You will get very biased answers just with how you formulated your
> question...
> 
> Could be summarise with: 15s sucks, but 5s is much better. Which one would
> you prefer? :)

I don't think I have *that* influence on people. I talked enough about a seek amount of 10 s and said anything goes.

(In reply to Jean-Yves Avenard [:jya] from comment #12)
> YouTube has both 10s and 5s seeking shortcuts.
> 
> So I'd say 10s is better, as jumping by 5s at a time, even with a repeat key
> would make quick travel in a video particularly slow.

I don't think touch-event-based seeking is of any importance in this discussion.

YouTube on a desktop computer seeks 5 s with bare left/right keys. (No modifier key made for a different seek amount in my test.)

> I think 15s forward and 5s forward would be ideal: you can find the scene
> you want quickly, and if you went too far, just have more control going
> backward...

How about this? @everyone

- No pref
- These key definitions:
    - MODIFIER 1+ARROW KEY: frame-wise seeking
        - `,` and `.` keys on YouTube
        - Ctrl+Arrow Key in MPC-BE
        - Ctrl as modifier would map to Command on Mac
        - Currently, only `seekToNextFrame()` is utilizable (no `seekToPreviousFrame()`),
            but it's a straightforward key definition and `...Previous...` could come
            in the future.
    - ARROW KEY: 5 s
        - "What did he say? *quickly hammering on single key*"
    - MODIFIER 2+ARROW KEY: 15/20 s
        - MPC-BE has 20 s as default big jump (no default key binding). Would coincide
            with triple-tapping in YouTube mobile app.
        - Shift as modifier? (BIG jump like UPPERcase letter. Like lower- and uppercase
            letters are related, the fixed seek amounts are related unlike a seek amount
            of 10%.)
    - MODIFIER 3+ARROW KEY: 10%
        - Similar to using bare digit keys on YouTube, but no absolute seek destinations,
            instead relative to current time (no behavior change).
        - Alt as modifier?

Comment 16

[Jean-Yves Avenard [:jya]]

Don't see the need to make this preference based.. not touching HTMLMediaElement is preferred.

Comment 17

[h-h]

Any opinions on my suggestion?

Comment 18

[:Gijs (he/him)]

(In reply to h-h from comment #17)
> Any opinions on my suggestion?

I don't think we need (or should build + maintain) 4 different ways of searching through a video. What's wrong with the proposal from comment #12 to use 15s forward, 5s back, and keep the existing way of seeking 10%?

Comment 19

[h-h]

(In reply to :Gijs (he/him) from comment #18)
> I don't think we need (or should build + maintain) 4 different ways of
> searching through a video. What's wrong with the proposal from comment #12
> to use 15s forward, 5s back, and keep the existing way of seeking 10%?

I'll write it. And then, there's barely anything to maintain. The number of lines of code should not differ much, because the current code is too verbose.

Why is `seekToNextFrame()` there if it's not meant to be used, if users are not meant to benefit from it?

If Firefox functions as a video player, it's appropriate to establish different seek amounts just like in every other video player.

I wouldn't want to have different seek amounts between seeking backward and seeking forward. That's not usual. And not appropriate IMO. And 15 seconds is too much.

Comment 20

[Jean-Yves Avenard [:jya]]

There's much more to maintenance than writing the initial patch...

I agree with :Gijs above.

Comment 21

[h-h]

The people at /r/firefox were generally for a seek amount of 5 seconds.

Since you are against a pref, I'd like to change the fixed non-relative seek amount to 5 seconds, both backward and forward. For faster seeking you have a seek amount of 10% or the mouse.

Comment 22

[tomek-kubel2]

Can we get an update? Even simple 2-line edit to 5 sec would be better than nothing after a year.

Comment 23

[h-h]

Attachment: Bug 1474149 - Shorten media seek amount to 5 seconds

When you play audio or video in Firefox, you can jump by a fixed amount of 15 seconds, as this help page explicitly mentions: https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly#w_media-shortcuts.

15 seconds is an unusual big seek step. These apps all have 5 seconds as their default seek step:

• YouTube
• foobar2000 (popular audio player for Windows)
• MPC-BE (video player for Windows)

A number of people thought, a config option as media.seekStepLength, that was initially planned, would not be necessary and proposed changing the fixed seek amount from 15 seconds to 5 seconds. That's what this patch does.

Here are media files for testing. You may have to click on the pause/play button or video area before the keystrokes will be recognized:

• Audio: http://wilwheaton.typepad.com/files/wil_wheaton_vs_text_2_speech.mp3
• Video:
  • https://www.w3schools.com/html/html5_video.asp
  • http://www.belleslettres.eu/video/konjunktiv-irrealis.mp4

Aside from that, the patch does the following:

• Improve the unit chaos (seconds vs. milliseconds), mostly by adding ms to variable names, since HTMLMediaElement is specced with properties in seconds, not milliseconds. I that carefully, not as a simple find-and-replace.
• Remove inconsistency: ==/!= to ===/!== (thoughtfully)

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Comment 25

[Bros123]

Since 3 years have passed. Is there a way now to customize the button foward and back in the default video player of firefox? maybe on about config

Comment 26

[:Gijs (he/him)]

(In reply to dragons.fire.zx from comment #25)

Since 3 years have passed. Is there a way now to customize the button foward and back in the default video player of firefox? maybe on about config

No, but the default seek time was changed to 5s in bug 1693961, which fulfilled the original request here.

Comment 27

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 28

[David Charlap]

FWIW, I would also like this seek amount to be configurable. And I think the reason for not making it configurable (the UI hard-coded the duration in the bitmap for the button), is not a good excuse.

Skip durations are a matter of personal preference, and are usually configurable on set-top-box media players like DVRs for exactly this reason.

FWIW, my personal preference is for skip-forward to skip 30 seconds, and skip-backward to skip 10 seconds. So you can quickly scrub forward through a video, and then back-off a bit if you overshoot the mark.

But others (like the OP for this bug report) want 5 seconds in both directions. And I'm sure there are dozens of other common preferences.

I think the duration definitely needs to be made configurable. Maybe provide a list of several choices in the Settings page, which can be selected for each direction, and make it an integer property in about:config, so those unsatisfied with the choices can pick something else.