Bug 27598 – Clean up the AccessibilityObject class

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Beth Dakin]

The AccessibilityObject class is the base class for all types accessibility objects. It has a lot of empty stubs of functions so that the main client of AccessibilityObject -- the AccessibilityObjectWrapper -- does not have to cast to different types of AccessibilityObjects to call appropriate functions. Some of these stubs should be pure virtual since all of the sub-classes implement them anyway. Others will need to stay empty stubs to avoid casting, but we can still clean up the class by moving them all to the header file since only half of these stubs are there now.

[Beth Dakin]

Created attachment 33316 [details]
Patch

[Darin Adler]

Comment on attachment 33316 [details]
Patch

Making the functions pure virtual seems great to me!

It also seems OK to move these "stubs" to the header. I know Hyatt likes this style.

But personally I normally avoid putting bodies of virtual functions into the class definition in the header, even small stubs. For one thing, it means changing any of these requires recompilation of any files that include the header. And generally there are few opportunities for virtual functions to be inlined, which is the usual reason for putting things in a header or in the class definition.

> -    virtual VisiblePosition visiblePositionForIndex(unsigned indexValue, bool lastIndexOK) const;
> +    virtual VisiblePosition visiblePositionForIndex(unsigned, bool) const { return VisiblePosition(); }

Here I think it's not so great to lose the names of the arguments, since the meaning of the bool is not clear without a name. Maybe keep lastIndexOK around, but commented out, like this:

    virtual VisiblePosition visiblePositionForIndex(unsigned, bool /* lastIndexOK */) const { return VisiblePosition(); }

Or leave this one in the implementation file.

r=me as-is, but please consider the lastIndexOK thing

[Beth Dakin]

Thanks Darin! I fixed the lastIndexOK thing.

Fixed with r46276.