[webkit-reviews] review granted: [Bug 171715] AX: Media Controls: Missing labels for the Time Labels. : [Attachment 312166] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 7 01:32:20 PDT 2017


Antoine Quint <graouts at apple.com> has granted Aaron Chu <aaron_chu at apple.com>'s
request for review:
Bug 171715: AX: Media Controls: Missing labels for the Time Labels.
https://bugs.webkit.org/show_bug.cgi?id=171715

Attachment 312166: Patch

https://bugs.webkit.org/attachment.cgi?id=312166&action=review




--- Comment #10 from Antoine Quint <graouts at apple.com> ---
Comment on attachment 312166
  --> https://bugs.webkit.org/attachment.cgi?id=312166
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312166&action=review

> Source/WebCore/Modules/modern-media-controls/controls/time-label.js:74
> +	       this.element.setAttribute("aria-label", `${ariaLabel}:
${timeAsString}`);

You can use `const` for these two since they don't change.

> LayoutTests/media/modern-media-controls/time-label/time-label.html:18
> +timeLabel.element.setAttribute("id", "elasped");

This is fine but you can shorten this to `timeLabel.element.id = "elapsed"`.

> LayoutTests/media/modern-media-controls/time-label/time-label.html:27
> +remainingTimeLabel.element.setAttribute("id", "remaining");

Same here about using the "id" property directly.

> LayoutTests/media/modern-media-controls/time-label/time-label.html:52
> +    finishMediaControlsTest(); 

Please remove that extra white space.


More information about the webkit-reviews mailing list