RESOLVED FIXED 85842
[Chromium] Android wishes to use an empty implementation if AXObjectCache 
https://bugs.webkit.org/show_bug.cgi?id=85842
Summary [Chromium] Android wishes to use an empty implementation if AXObjectCache
Adam Barth
Reported 2012-05-07 16:29:10 PDT
[Chromium] Android wishes to use an empty implementation if AXObjectCache
Attachments
Patch (7.94 KB, patch)
2012-05-07 16:30 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (3.05 KB, patch)
2012-05-07 17:43 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-05-07 16:30:38 PDT
Created attachment 140618 [details] Patch
Tony Chang
Comment 2 2012-05-07 16:45:06 PDT
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
Comment 3 2012-05-07 16:50:26 PDT
(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)
Comment 4 2012-05-07 16:53:39 PDT
Why do they even want an AXObjectCache? Seems they just want to disable ACCESSIBILITY. :)
Adam Barth
Comment 5 2012-05-07 17:00:57 PDT
> 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)
Comment 6 2012-05-07 17:02:06 PDT
I suspect they may have ended up this route, because the PLATFORM(CHROMIUM) && !HAVE(ACCESSIBILITY) path may not build cleanly.
Eric Seidel (no email)
Comment 7 2012-05-07 17:02:47 PDT
Of course disabling HAVE(ACCESSIBILITY) could make their binary smaller too. :)
Adam Barth
Comment 8 2012-05-07 17:43:00 PDT
Created attachment 140637 [details] Patch
Eric Seidel (no email)
Comment 9 2012-05-07 17:48:11 PDT
Comment on attachment 140637 [details] Patch LGTM.
WebKit Review Bot
Comment 10 2012-05-07 19:46:31 PDT
Comment on attachment 140637 [details] Patch Clearing flags on attachment: 140637 Committed r116385: <http://trac.webkit.org/changeset/116385>
WebKit Review Bot
Comment 11 2012-05-07 19:46:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.