Skip to content
Permalink
Browse files
[Modern Media Controls] [macOS] Only show total duration when the ela…
…psed time is visible

https://bugs.webkit.org/show_bug.cgi?id=228191
<rdar://problem/80529918>

Reviewed by Eric Carlson.

Source/WebCore:

When a `<video>` is narrow (~250px) WebKit will drop the elapsed time, meaning that only the
scrubber and duration are visible. It's not very helpful to show just the duration because
it never changes, requiring the user to do the mental math of dividing the scrubber width
by the total duration to figure out how much time is left (and has elapsed). Showing the
remaining time is better because it at least helps the user not have to figure out how much
longer the `<video>` will play (which is more useful/actionable than knowing the duration).

Test: media/modern-media-controls/time-control/time-control.html

* Modules/modern-media-controls/controls/time-control.js:
(TimeControl):
(TimeControl.prototype.handleEvent):
(TimeControl.prototype.get _canShowDurationTimeLabel): Added.
(TimeControl.prototype._durationOrRemainingTimeLabel):
(TimeControl.prototype._performIdealLayout):
Rename `_showDurationTimeLabel` to `_shouldShowDurationTimeLabel` and add another private
getter `_canShowDurationTimeLabel` that only is true when `this.elapsedTimeLabel.visible`.
Use both to decide whether duration or remaining is shown/updated/etc..

LayoutTests:

* media/modern-media-controls/time-control/time-control.html:
* media/modern-media-controls/time-control/time-control-expected.txt:


Canonical link: https://commits.webkit.org/239910@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280242 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dcrousso committed Jul 23, 2021
1 parent 423aa6c commit 2e37a2d5226d84cf3986cdb2e185d5bfa2869efc
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 13 deletions.
@@ -1,3 +1,14 @@
2021-07-23 Devin Rousso <drousso@apple.com>

[Modern Media Controls] [macOS] Only show total duration when the elapsed time is visible
https://bugs.webkit.org/show_bug.cgi?id=228191
<rdar://problem/80529918>

Reviewed by Eric Carlson.

* media/modern-media-controls/time-control/time-control.html:
* media/modern-media-controls/time-control/time-control-expected.txt:

2021-07-23 Eleni Maria Stea <estea@igalia.com>

Removed incorrect test from webgl tests.
@@ -8,17 +8,62 @@ PASS timeControl.element.className is "time-control"

PASS timeControl.elapsedTimeLabel is an instance of TimeLabel
PASS timeControl.scrubber is an instance of Slider
PASS timeControl.durationTimeLabel is an instance of TimeLabel
PASS timeControl.remainingTimeLabel is an instance of TimeLabel

timeControl.width = 500;
PASS timeControl.children.length is 3
PASS timeControl.children[0] is timeControl.elapsedTimeLabel
PASS timeControl.children[1] is timeControl.scrubber
PASS timeControl.children[2] is timeControl.durationTimeLabel
PASS timeControl.elapsedTimeLabel.x is 0
PASS timeControl.elapsedTimeLabel.visible is true
PASS timeControl.scrubber.x is 32
PASS timeControl.scrubber.width is 436
PASS timeControl.durationTimeLabel.x is 473

timeControl.width = 100;
PASS timeControl.children.length is 3
PASS timeControl.children[0] is timeControl.elapsedTimeLabel
PASS timeControl.children[1] is timeControl.scrubber
PASS timeControl.children[2] is timeControl.remainingTimeLabel
PASS timeControl.elapsedTimeLabel.visible is false
PASS timeControl.scrubber.x is 0
PASS timeControl.scrubber.width is 62
PASS timeControl.remainingTimeLabel.x is 67

timeControl.width = 200;
PASS timeControl.children.length is 3
PASS timeControl.children[0] is timeControl.elapsedTimeLabel
PASS timeControl.children[1] is timeControl.scrubber
PASS timeControl.children[2] is timeControl.durationTimeLabel
PASS timeControl.elapsedTimeLabel.x is 0
PASS timeControl.elapsedTimeLabel.visible is true
PASS timeControl.scrubber.x is 32
PASS timeControl.scrubber.width is 136
PASS timeControl.durationTimeLabel.x is 173

timeControl.durationTimeLabel.element.click();
PASS timeControl.children.length is 3
PASS timeControl.children[0] is timeControl.elapsedTimeLabel
PASS timeControl.children[1] is timeControl.scrubber
PASS timeControl.children[2] is timeControl.remainingTimeLabel
PASS timeControl.elapsedTimeLabel.x is 0
PASS timeControl.elapsedTimeLabel.visible is true
PASS timeControl.scrubber.x is 32
PASS timeControl.scrubber.width is 130
PASS timeControl.remainingTimeLabel.x is 167

timeControl.remainingTimeLabel.element.click();
PASS timeControl.children.length is 3
PASS timeControl.children[0] is timeControl.elapsedTimeLabel
PASS timeControl.children[1] is timeControl.scrubber
PASS timeControl.children[2] is timeControl.durationTimeLabel
PASS timeControl.elapsedTimeLabel.x is 0
PASS timeControl.elapsedTimeLabel.visible is true
PASS timeControl.scrubber.x is 32
PASS timeControl.scrubber.width is 430
PASS timeControl.remainingTimeLabel.x is 467
PASS timeControl.scrubber.width is 136
PASS timeControl.durationTimeLabel.x is 173

PASS successfullyParsed is true

@@ -5,27 +5,85 @@

description("Testing the <code>TimeControl</code> class.");

const timeControl = new TimeControl({ layoutTraits: new IOSLayoutTraits(LayoutTraits.Mode.Inline) });
const timeControl = new TimeControl({ layoutTraits: new MacOSLayoutTraits(LayoutTraits.Mode.Inline) });

shouldBeEqualToString("timeControl.element.localName", "div");
shouldBeEqualToString("timeControl.element.className", "time-control");

debug("");
shouldBeType("timeControl.elapsedTimeLabel", "TimeLabel");
shouldBeType("timeControl.scrubber", "Slider");
shouldBeType("timeControl.durationTimeLabel", "TimeLabel");
shouldBeType("timeControl.remainingTimeLabel", "TimeLabel");

debug("");
debug("timeControl.width = 500;");
timeControl.width = 500;
scheduler.flushScheduledLayoutCallbacks();
shouldBe("timeControl.children.length", "3");
shouldBe("timeControl.children[0]", "timeControl.elapsedTimeLabel");
shouldBe("timeControl.children[1]", "timeControl.scrubber");
shouldBe("timeControl.children[2]", "timeControl.durationTimeLabel");
shouldBe("timeControl.elapsedTimeLabel.x", "0");
shouldBe("timeControl.elapsedTimeLabel.visible", "true");
shouldBe("timeControl.scrubber.x", "32");
shouldBe("timeControl.scrubber.width", "436");
shouldBe("timeControl.durationTimeLabel.x", "473");

debug("");
debug("timeControl.width = 100;");
timeControl.width = 100;
scheduler.flushScheduledLayoutCallbacks();
shouldBe("timeControl.children.length", "3");
shouldBe("timeControl.children[0]", "timeControl.elapsedTimeLabel");
shouldBe("timeControl.children[1]", "timeControl.scrubber");
shouldBe("timeControl.children[2]", "timeControl.remainingTimeLabel");
shouldBe("timeControl.elapsedTimeLabel.visible", "false");
shouldBe("timeControl.scrubber.x", "0");
shouldBe("timeControl.scrubber.width", "62");
shouldBe("timeControl.remainingTimeLabel.x", "67");

debug("");
debug("timeControl.width = 200;");
timeControl.width = 200;
scheduler.flushScheduledLayoutCallbacks();
shouldBe("timeControl.children.length", "3");
shouldBe("timeControl.children[0]", "timeControl.elapsedTimeLabel");
shouldBe("timeControl.children[1]", "timeControl.scrubber");
shouldBe("timeControl.children[2]", "timeControl.durationTimeLabel");
shouldBe("timeControl.elapsedTimeLabel.x", "0");
shouldBe("timeControl.elapsedTimeLabel.visible", "true");
shouldBe("timeControl.scrubber.x", "32");
shouldBe("timeControl.scrubber.width", "136");
shouldBe("timeControl.durationTimeLabel.x", "173");

debug("");
debug("timeControl.durationTimeLabel.element.click();");
timeControl.durationTimeLabel.element.click()
scheduler.flushScheduledLayoutCallbacks();
shouldBe("timeControl.children.length", "3");
shouldBe("timeControl.children[0]", "timeControl.elapsedTimeLabel");
shouldBe("timeControl.children[1]", "timeControl.scrubber");
shouldBe("timeControl.children[2]", "timeControl.remainingTimeLabel");
shouldBe("timeControl.elapsedTimeLabel.x", "0");
shouldBe("timeControl.elapsedTimeLabel.visible", "true");
shouldBe("timeControl.scrubber.x", "32");
shouldBe("timeControl.scrubber.width", "130");
shouldBe("timeControl.remainingTimeLabel.x", "167");

debug("");
debug("timeControl.remainingTimeLabel.element.click();");
timeControl.remainingTimeLabel.element.click();
scheduler.flushScheduledLayoutCallbacks();
shouldBe("timeControl.children.length", "3");
shouldBe("timeControl.children[0]", "timeControl.elapsedTimeLabel");
shouldBe("timeControl.children[1]", "timeControl.scrubber");
shouldBe("timeControl.children[2]", "timeControl.durationTimeLabel");
shouldBe("timeControl.elapsedTimeLabel.x", "0");
shouldBe("timeControl.elapsedTimeLabel.visible", "true");
shouldBe("timeControl.scrubber.x", "32");
shouldBe("timeControl.scrubber.width", "430");
shouldBe("timeControl.remainingTimeLabel.x", "467");
shouldBe("timeControl.scrubber.width", "136");
shouldBe("timeControl.durationTimeLabel.x", "173");

debug("");

@@ -1,3 +1,30 @@
2021-07-23 Devin Rousso <drousso@apple.com>

[Modern Media Controls] [macOS] Only show total duration when the elapsed time is visible
https://bugs.webkit.org/show_bug.cgi?id=228191
<rdar://problem/80529918>

Reviewed by Eric Carlson.

When a `<video>` is narrow (~250px) WebKit will drop the elapsed time, meaning that only the
scrubber and duration are visible. It's not very helpful to show just the duration because
it never changes, requiring the user to do the mental math of dividing the scrubber width
by the total duration to figure out how much time is left (and has elapsed). Showing the
remaining time is better because it at least helps the user not have to figure out how much
longer the `<video>` will play (which is more useful/actionable than knowing the duration).

Test: media/modern-media-controls/time-control/time-control.html

* Modules/modern-media-controls/controls/time-control.js:
(TimeControl):
(TimeControl.prototype.handleEvent):
(TimeControl.prototype.get _canShowDurationTimeLabel): Added.
(TimeControl.prototype._durationOrRemainingTimeLabel):
(TimeControl.prototype._performIdealLayout):
Rename `_showDurationTimeLabel` to `_shouldShowDurationTimeLabel` and add another private
getter `_canShowDurationTimeLabel` that only is true when `this.elapsedTimeLabel.visible`.
Use both to decide whether duration or remaining is shown/updated/etc..

2021-07-23 Philippe Normand <pnormand@igalia.com>

[GLib] Remove libportal dependency
@@ -39,9 +39,11 @@ class TimeControl extends LayoutItem
layoutDelegate
});

this._shouldShowDurationTimeLabel = this.layoutTraits.supportsDurationTimeLabel();

this.elapsedTimeLabel = new TimeLabel(TimeLabel.Type.Elapsed);
this.scrubber = new Slider("scrubber", this.layoutTraits.knobStyleForScrubber());
if (this.layoutTraits.supportsDurationTimeLabel())
if (this._shouldShowDurationTimeLabel)
this.durationTimeLabel = new TimeLabel(TimeLabel.Type.Duration);
this.remainingTimeLabel = new TimeLabel(TimeLabel.Type.Remaining);

@@ -55,8 +57,7 @@ class TimeControl extends LayoutItem
this._currentTime = 0;
this._loading = false;

this._showDurationTimeLabel = this.layoutTraits.supportsDurationTimeLabel();
if (this._showDurationTimeLabel) {
if (this._shouldShowDurationTimeLabel) {
this.durationTimeLabel.element.addEventListener("click", this);
this.remainingTimeLabel.element.addEventListener("click", this);
}
@@ -140,23 +141,30 @@ class TimeControl extends LayoutItem
case "click":
switch (event.target) {
case this.durationTimeLabel.element:
this._showDurationTimeLabel = false;
this._shouldShowDurationTimeLabel = false;
this.needsLayout = true;
break;

case this.remainingTimeLabel.element:
this._showDurationTimeLabel = true;
this.needsLayout = true;
if (this._canShowDurationTimeLabel) {
this._shouldShowDurationTimeLabel = true;
this.needsLayout = true;
}
break;
}
}
}

// Private

get _canShowDurationTimeLabel()
{
return this.elapsedTimeLabel.visible;
}

_durationOrRemainingTimeLabel()
{
return this._showDurationTimeLabel ? this.durationTimeLabel : this.remainingTimeLabel;
return (this._canShowDurationTimeLabel && this._shouldShowDurationTimeLabel) ? this.durationTimeLabel : this.remainingTimeLabel;
}

_performIdealLayout()
@@ -177,7 +185,7 @@ class TimeControl extends LayoutItem
numberOfDigitsForTimeLabels = 6;

this.elapsedTimeLabel.setValueWithNumberOfDigits(shouldShowZeroDurations ? 0 : this._currentTime, numberOfDigitsForTimeLabels);
if (this._showDurationTimeLabel)
if (this._canShowDurationTimeLabel && this._shouldShowDurationTimeLabel)
this.durationTimeLabel.setValueWithNumberOfDigits(shouldShowZeroDurations ? 0 : this._duration, numberOfDigitsForTimeLabels);
else
this.remainingTimeLabel.setValueWithNumberOfDigits(shouldShowZeroDurations ? 0 : this._currentTime - this._duration, numberOfDigitsForTimeLabels);

0 comments on commit 2e37a2d

Please sign in to comment.