Bug 173858 – [Modern Media Controls] Accessibility labels should be formatted using NSDateComponentsFormatter

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Ryosuke Niwa]

Media control accessibility text should use stringsdict

[Ryosuke Niwa]

Created attachment 313889 [details]
Make media control use stringsdict

[Ryosuke Niwa]

Comment on attachment 313889 [details]
Make media control use stringsdict

Actually, this doesn't work. The modern media controls doesn't use the localized strings at all :(

[Ryosuke Niwa]

<rdar://problem/32643171>

[Antoine Quint]

I think the right fix is to use NSDateComponentsFormatter, so retitling this bug to cover this.

[Antoine Quint]

Created attachment 313913 [details]
Patch

[Build Bot]

Comment on attachment 313913 [details]
Patch

Attachment 313913 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4006504

New failing tests:
media/modern-media-controls/scrubber/scrubber-has-correct-ax-label.html

[Build Bot]

Created attachment 313917 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313913 [details]
Patch

Attachment 313913 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4006514

New failing tests:
media/modern-media-controls/scrubber/scrubber-has-correct-ax-label.html

[Build Bot]

Created attachment 313918 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313913 [details]
Patch

Attachment 313913 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4006538

New failing tests:
media/modern-media-controls/scrubber/scrubber-has-correct-ax-label.html

[Build Bot]

Created attachment 313920 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Antoine Quint]

Created attachment 313921 [details]
Patch

[Antoine Quint]

Created attachment 313924 [details]
Patch

[Dean Jackson]

Comment on attachment 313924 [details]
Patch

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

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292
> +String MediaControlsHost::formattedDuration(const double durationInSeconds) const

I'm not sure about this name. Maybe formattedStringForDuration? or just stringForDuration?

> Source/WebCore/rendering/RenderTheme.h:97
> +    virtual String mediaControlsFormattedDuration(const double) { return String(); }

Ditto here.

> Source/WebCore/rendering/RenderThemeMac.mm:309
> +String RenderThemeMac::mediaControlsFormattedDuration(const double durationInSeconds)

We don't have a RenderThemeCocoa?

[Tim Horton]

(In reply to Dean Jackson from comment #14)
> Comment on attachment 313924 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313924&action=review
> 
> > Source/WebCore/rendering/RenderThemeMac.mm:309
> > +String RenderThemeMac::mediaControlsFormattedDuration(const double durationInSeconds)
> 
> We don't have a RenderThemeCocoa?

We do!

[Tim Horton]

Comment on attachment 313924 [details]
Patch

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

>> Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292
>> +String MediaControlsHost::formattedDuration(const double durationInSeconds) const
> 
> I'm not sure about this name. Maybe formattedStringForDuration? or just stringForDuration?

userVisibleDuration :P

[Antoine Quint]

Created attachment 313943 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 313943 [details]
Patch for landing

Clearing flags on attachment: 313943

Committed r218853: <http://trac.webkit.org/changeset/218853>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 313943 [details]
Patch for landing

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

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292
> +String MediaControlsHost::formattedStringForDuration(const double durationInSeconds) const

The const here is basically meaningless and not needed.

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.h:91
> +    String formattedStringForDuration(const double) const;

Ditto.

[Antoine Quint]

(In reply to Darin Adler from comment #20)
> Comment on attachment 313943 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313943&action=review
> 
> > Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292
> > +String MediaControlsHost::formattedStringForDuration(const double durationInSeconds) const
> 
> The const here is basically meaningless and not needed.
> 
> > Source/WebCore/Modules/mediacontrols/MediaControlsHost.h:91
> > +    String formattedStringForDuration(const double) const;
> 
> Ditto.

Raised https://bugs.webkit.org/show_bug.cgi?id=173925 for the followup.