[Webkit-unassigned] [Bug 177788] AX: Make video objects accessible on iOS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=177788

chris fleizach <cfleizach at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cfleizach at apple.com

--- Comment #3 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 322461
  --> https://bugs.webkit.org/attachment.cgi?id=322461
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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171004/fa9d01ea/attachment-0001.html>