AX: Make video objects accessible on iOS

[Nan Wang]

We should expose the video object itself so that we are able to provide some general functions to let iOS users interact with the video playback.

[Radar WebKit Bug Importer]

<rdar://problem/34778028>

[Nan Wang]

Created attachment 322461 [details]
Patch

[chris fleizach]

Comment on attachment 322461 [details]
Patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:449
> +    if (node && is<HTMLMediaElement>(node) && nodeHasRole(node, nullAtom()))

I think the is<> check takes care of node == null case, so you can remove that

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:66
> +    if (!node || !is<HTMLMediaElement>(*node))

I think the is<> check takes care of !node

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:73
> +    if (HTMLMediaElement* element = mediaElement()) {

this block can be done in two lines

if (HTMLMediaElement* element = mediaElement())
   return localizedMediaTimeDescription(element->currentTime())

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:82
> +    if (HTMLMediaElement* element = mediaElement()) {

same comment as above

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:89
> +void AccessibilityMediaObject::fastSeek(bool forward)

fast doesn't seem like the right descriptor here. 

is this just mediaSeek() or is there something faster about this

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:95
> +    (void)forward;

I think you can use UNUSED_PARAM(forward)

> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:100
> +    double timeDelta = ceil(duration * 5.0 / 100.0);

I think we can just do

const double seekStep = .05;
and use that rather than doing the math

> Source/WebCore/accessibility/AccessibilityMediaObject.h:42
> +    String stringValue() const override;

most of these might be able to be moved to private: since they're overrides

> Source/WebCore/accessibility/AccessibilityMediaObject.h:65
> +    void fastSeek(bool);

instead of using a bool, you might use

enum { SeekForward, SeekBackward }

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:686
> +    return downcast<AccessibilityMediaObject>(m_object)->duration();

returning the duration for a "videoDescription" seems like a mismatch. maybe this should be named "interactiveVideoDuration"

> LayoutTests/accessibility/ios-simulator/video-elements-ios.html:10
> +  Your browser does not support the video tag.

can you add some of the video cases where we don't want to expose as an object to verify that

[Nan Wang]

Comment on attachment 322461 [details]
Patch

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

Thanks, will address them.

>> Source/WebCore/accessibility/AccessibilityMediaObject.cpp:95
>> +    (void)forward;
> 
> I think you can use UNUSED_PARAM(forward)

Oh I accidentally left this in.

[Nan Wang]

Created attachment 322735 [details]
patch

update from review

[WebKit Commit Bot]

Comment on attachment 322735 [details]
patch

Clearing flags on attachment: 322735

Committed r222887: <http://trac.webkit.org/changeset/222887>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Matt Lewis]

This patch introduced a test that is failing consistently on iOS:

accessibility/ios-simulator/video-elements-ios.html


https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Fios-simulator%2Fvideo-elements-ios.html

build:

https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r222911%20(343)/results.html
https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/builds/343

[Nan Wang]

(In reply to Matt Lewis from comment #8)
> This patch introduced a test that is failing consistently on iOS:
> 
> accessibility/ios-simulator/video-elements-ios.html
> 
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=accessibility%2Fios-simulator%2Fvideo-elements-
> ios.html
> 
> build:
> 
> https://build.webkit.org/results/
> Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r222911%20(343)/
> results.html
> https://build.webkit.org/builders/
> Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/builds/343

Ok I'll look into this in
https://bugs.webkit.org/show_bug.cgi?id=177954