Summary: | [Chromium] Android wishes to use an empty implementation if AXObjectCache | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric, johnme, peter, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 66687 | ||||||||
Attachments: |
|
Description
Adam Barth
2012-05-07 16:29:10 PDT
Created attachment 140618 [details]
Patch
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. (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. Why do they even want an AXObjectCache? Seems they just want to disable ACCESSIBILITY. :) > 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.
I suspect they may have ended up this route, because the PLATFORM(CHROMIUM) && !HAVE(ACCESSIBILITY) path may not build cleanly. Of course disabling HAVE(ACCESSIBILITY) could make their binary smaller too. :) Created attachment 140637 [details]
Patch
Comment on attachment 140637 [details]
Patch
LGTM.
Comment on attachment 140637 [details] Patch Clearing flags on attachment: 140637 Committed r116385: <http://trac.webkit.org/changeset/116385> All reviewed patches have been landed. Closing bug. |