Bug 160619 – AX: Media controls timeline should have percentage value description

• Component: Accessibility
• Status: RESOLVED FIXED

[Nan Wang]

VoiceOver is speaking the seconds value when landing on the timeline control.

<rdar://problem/27645753>

[Nan Wang]

Created attachment 285461 [details]
patch

[chris fleizach]

Comment on attachment 285461 [details]
patch

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

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1581
> +            return `${intHours} ${this.UIString('Hours')}, ${intMinutes} ${this.UIString('Minutes')}, ${intSeconds} ${this.UIString('Seconds')}`;

Does this take care of singular vs. Plural

> LayoutTests/platform/efl/accessibility/media-element-expected.txt:24
> +            title: AXTitle: Elapsed 0 Seconds

We should add cases for minutes and hours as well
As well as singular vs. Plural

[Nan Wang]

Comment on attachment 285461 [details]
patch

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

>> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1581
>> +            return `${intHours} ${this.UIString('Hours')}, ${intMinutes} ${this.UIString('Minutes')}, ${intSeconds} ${this.UIString('Seconds')}`;
> 
> Does this take care of singular vs. Plural

Nope. Will add that.

>> LayoutTests/platform/efl/accessibility/media-element-expected.txt:24
>> +            title: AXTitle: Elapsed 0 Seconds
> 
> We should add cases for minutes and hours as well
> As well as singular vs. Plural

Ok, will do.

[Nan Wang]

Comment on attachment 285461 [details]
patch

Going to fix test and review comments.

[Nan Wang]

Created attachment 285511 [details]
patch

Added code to cover singular and plural time units. Also added test cases. 
Not sure how to add test cases to cover minutes and hours without including a large video.

[Eric Carlson]

(In reply to comment #5)
> Created attachment 285511 [details]
> patch
> 
> Added code to cover singular and plural time units. Also added test cases. 
> Not sure how to add test cases to cover minutes and hours without including
> a large video.

You could generate a VOD HLS stream with a little bit of PHP.  LayoutTests/http/tests/media/resources/hls/test-live.php generates a live stream, but you should be able to do something similar that generates a fixed duration stream based on a parameter passed from script.

[Nan Wang]

(In reply to comment #6)
> (In reply to comment #5)
> > Created attachment 285511 [details]
> > patch
> > 
> > Added code to cover singular and plural time units. Also added test cases. 
> > Not sure how to add test cases to cover minutes and hours without including
> > a large video.
> 
> You could generate a VOD HLS stream with a little bit of PHP. 
> LayoutTests/http/tests/media/resources/hls/test-live.php generates a live
> stream, but you should be able to do something similar that generates a
> fixed duration stream based on a parameter passed from script.

Thanks Eric! Will look into that.

[Nan Wang]

Created attachment 285708 [details]
patch

updated tests

[Eric Carlson]

Comment on attachment 285708 [details]
patch

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

> LayoutTests/ChangeLog:10
> +        * http/tests/media/resources/hls/test-video-duration-accessibility.php: Added.

This file isn't accessibility-specific, how about something like "generate-vod.php"?

> LayoutTests/http/tests/media/hls/video-duration-accessibility.html:36
> +                // We are not getting the exact same elapsed duration for hour cases, 
> +                // so let's special case this.

What does this mean, is it something we need to fix?

> LayoutTests/http/tests/media/hls/video-duration-accessibility.html:42
> +                if (elapsedTimer.description.indexOf("Hour") !== -1) {
> +                    testExpected("elapsedTimer.description.indexOf('1 Hour') !== -1 || elapsedTimer.description.indexOf('2 Hours') !== -1", true);
> +                } else {
> +                    consoleWrite("elapsedTimer.description: " + elapsedTimer.description);
> +                }
> +                

Nit: WebKit style is to not use braces for single line if statements.

> LayoutTests/http/tests/media/hls/video-duration-accessibility.html:47
> +                if (seekCount == seekTimes.length) {
> +                    endTest();
> +                } else {
> +                    video.fastSeek(seekTimes[seekCount]);
> +                }

Ditto.

> LayoutTests/http/tests/media/resources/hls/test-video-duration-accessibility.php:10
> +// header("Content-Length: " . filesize(__FILE__));

Nit: this isn't necessary.

> LayoutTests/http/tests/media/resources/hls/test-video-duration-accessibility.php:13
> +$chunkDuration = 6.0272;
> +$chunkCount = 1300;

I would prefer that the duration of the file come from an optional parameter so we can use this for other tests in the future. Something like (untested):

    $chunkCount = 1300;
    if (array_key_exists("duration", $_GET))
        $chunkCount = $_GET["duration"] / $chunkDuration;

[Nan Wang]

Comment on attachment 285708 [details]
patch

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

Thanks for the review, will address those issues.

>> LayoutTests/http/tests/media/hls/video-duration-accessibility.html:36
>> +                // so let's special case this.
> 
> What does this mean, is it something we need to fix?

So when I have the fixed large number passed into fastSeek(), like 3600, I'm not getting the same elapsed time each time when I run the test. I guess it's something to do with the #EXTINF is not exactly the same as the duration of the .ts file?

[Nan Wang]

Created attachment 285734 [details]
patch

Updated from review

[Build Bot]

Comment on attachment 285734 [details]
patch

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

New failing tests:
http/tests/media/hls/video-duration-accessibility.html

[Build Bot]

Created attachment 285744 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

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

[Eric Carlson]

Comment on attachment 285734 [details]
patch

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

> LayoutTests/http/tests/media/resources/hls/generate-vod.php:14
> +$chunkCount = 0;
> +if (array_key_exists("duration", $_GET)) 
> +    $chunkCount = $_GET["duration"] / $chunkDuration;

This makes the "duration" param optional, so $chunkCount should have a default value that will result in a playable stream.

[Nan Wang]

Created attachment 285745 [details]
Patch

Added default value for chunkCount.

[Nan Wang]

(In reply to comment #14)
> Comment on attachment 285734 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285734&action=review
> 
> > LayoutTests/http/tests/media/resources/hls/generate-vod.php:14
> > +$chunkCount = 0;
> > +if (array_key_exists("duration", $_GET)) 
> > +    $chunkCount = $_GET["duration"] / $chunkDuration;
> 
> This makes the "duration" param optional, so $chunkCount should have a
> default value that will result in a playable stream.

Seems this test is not passing on mac-wk2. But it passed on my local machine. Do you have any idea what could be the cause? The previous patch without the duration parameter seemed working fine.

[Build Bot]

Comment on attachment 285745 [details]
Patch

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

New failing tests:
http/tests/media/hls/video-duration-accessibility.html

[Build Bot]

Created attachment 285750 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

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

[Nan Wang]

(In reply to comment #16)
> (In reply to comment #14)
> > Comment on attachment 285734 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=285734&action=review
> > 
> > > LayoutTests/http/tests/media/resources/hls/generate-vod.php:14
> > > +$chunkCount = 0;
> > > +if (array_key_exists("duration", $_GET)) 
> > > +    $chunkCount = $_GET["duration"] / $chunkDuration;
> > 
> > This makes the "duration" param optional, so $chunkCount should have a
> > default value that will result in a playable stream.
> 
> Seems this test is not passing on mac-wk2. But it passed on my local
> machine. Do you have any idea what could be the cause? The previous patch
> without the duration parameter seemed working fine.

There seemed to be a stalled event when it tried to seek for 1 hour duration.

[Nan Wang]

(In reply to comment #18)
> Created attachment 285750 [details]
> Archive of layout-test-results from ews104 for mac-yosemite-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-wk2-ews.
> Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

For the new failure we are not getting the exact same minute duration as expected.

[Nan Wang]

Created attachment 285755 [details]
patch

Updated test

[Eric Carlson]

Comment on attachment 285755 [details]
patch

Thanks for keeping this Nan!

[Nan Wang]

Committed r204361: <http://trac.webkit.org/changeset/204361>