[Chromium] Android wishes to use an empty implementation if AXObjectCache

[Adam Barth]

[Chromium] Android wishes to use an empty implementation if AXObjectCache

[Adam Barth]

Created attachment 140618 [details]
Patch

[Tony Chang]

Comment on attachment 140618 [details]
Patch

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

> Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:76
> +bool nodeHasRole(Node*, const String& role)
> +{
> +    UNUSED_PARAM(role);

Nit: Can you just remove |role| on line 74 and not have to call UNUSED_PARAM?

> Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:150
> +void AXObjectCache::postNotification(RenderObject*, AXNotification, bool postToElement, PostType)
> +{
> +    UNUSED_PARAM(postToElement);
> +}

Ditto.

> Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:155
> +void AXObjectCache::postNotification(AccessibilityObject*, Document*, AXNotification, bool postToElement, PostType)
> +{
> +    UNUSED_PARAM(postToElement);
> +}

Ditto.

> Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:168
> +void AXObjectCache::nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned offset, const String&)
> +{
> +    UNUSED_PARAM(offset);
> +}

Ditto.

> Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:200
> +void AXObjectCache::handleFocusedUIElementChanged(RenderObject*, RenderObject* newFocusedRenderer)
> +{
> +    UNUSED_PARAM(newFocusedRenderer);
> +}

Ditto.

[Adam Barth]

(In reply to comment #2)
> (From update of attachment 140618 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140618&action=review
> 
> > Source/WebCore/accessibility/chromium/AXObjectCacheChromiumAndroid.cpp:76
> > +bool nodeHasRole(Node*, const String& role)
> > +{
> > +    UNUSED_PARAM(role);
> 
> Nit: Can you just remove |role| on line 74 and not have to call UNUSED_PARAM?

I can if you like.  I left in the parameters names for parameters that weren't obvious to me.  Maybe I should just remove them all given that this file is just an empty implementation.

[Eric Seidel (no email)]

Why do they even want an AXObjectCache?  Seems they just want to disable ACCESSIBILITY. :)

[Adam Barth]

> Seems they just want to disable ACCESSIBILITY. :)

It might be possible to go that route.  I know that Chrome uses some accessibility features internally, for example for testing.  There does appear to be a HAVE(ACCESSIBILITY) flag and some evidence that it can be turned off.

[Eric Seidel (no email)]

I suspect they may have ended up this route, because the PLATFORM(CHROMIUM) && !HAVE(ACCESSIBILITY) path may not build cleanly.

[Eric Seidel (no email)]

Of course disabling HAVE(ACCESSIBILITY) could make their binary smaller too. :)

[Adam Barth]

Created attachment 140637 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 140637 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 140637 [details]
Patch

Clearing flags on attachment: 140637

Committed r116385: <http://trac.webkit.org/changeset/116385>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.