Bug 57955

Summary: [GTK] Do not reference AccessibilityRenderObject from platform dependent code
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 52148    
Attachments:
Description Flags
Patch proposal cfleizach: review+

Mario Sanchez Prada
Reported 2011-04-06 08:55:35 PDT
Currently, many explicit references to AccessibilityRenderObject are being carried out in platform dependant code (that is, the GTK a11y wrapper and its related files), which shouldn't be needed if we had some methods virtually defined in AccessibilityObject.h, and properly overriden in its subclases. More specifically, the methods that we would need in AccessibilityObject.h to be able to get rid of those explicit references in the GTK wrapper would be the following two: virtual RenderObject* renderer() const { return 0; } virtual AccessibilityObject* correspondingLabelForControlElement() const { return 0; } Once defined, a lot of code could be simplified, and lines '#include "AccessibilityRenderObject.h"' removed from those .cpp files inside Source/WebCore/accessibility/gtk
Attachments
Patch proposal (26.84 KB, patch)
2011-04-06 09:12 PDT, Mario Sanchez Prada 		cfleizach: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1 2011-04-06 09:12:37 PDT
Created attachment 88443 [details] Patch proposal Here is a patch for this. It doesn't has a Layout test since the best test it doesn't add anything new. On top of that, I've run all the a11y tests in the GTK and mac platforms and they keep passing, so I assume I didn't break anything with it.
chris fleizach
Comment 2 2011-04-06 09:16:58 PDT
Comment on attachment 88443 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=88443&action=review looks good modulus changes > Source/WebCore/accessibility/AccessibilityObject.h:359 > + I would put this directly below node(), with no extra line between them, because they are related > Source/WebCore/accessibility/AccessibilityObject.h:410 > virtual AccessibilityObject* correspondingControlForLabelElement() const { return 0; } does correspondingLabelForControlElement need to be made virtual in AXRenderObject.h?
Mario Sanchez Prada
Comment 3 2011-04-06 09:36:47 PDT
(In reply to comment #2) > (From update of attachment 88443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88443&action=review > > looks good modulus changes > > > Source/WebCore/accessibility/AccessibilityObject.h:359 > > + > > I would put this directly below node(), with no extra line between them, > because they are related Ok, I'll change that before committing. > > Source/WebCore/accessibility/AccessibilityObject.h:410 > > virtual AccessibilityObject* correspondingControlForLabelElement() const { return 0; } > > does correspondingLabelForControlElement need to be made virtual in > AXRenderObject.h? Nope, as it is already virtual in there :-) Thanks for the review. Committing...
Mario Sanchez Prada
Comment 4 2011-04-06 09:50:42 PDT
Committed r83064: <http://trac.webkit.org/changeset/83064>
Note You need to log in before you can comment on or make changes to this bug.