[Webkit-unassigned] [Bug 33923] REGRESSION (Safari 4): AXValueChanged no longer sent for text area scrollbars

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 20 16:29:50 PST 2010


https://bugs.webkit.org/show_bug.cgi?id=33923





--- Comment #3 from Darin Adler <darin at apple.com>  2010-01-20 16:29:50 PST ---
(From update of attachment 47075)
I know Oliver already reviewed by I have a few comments and questions.

> +void AXObjectCache::postNotification(AccessibilityObject* obj, AXNotification notification, bool postToElement, PostType postType)

New code should use "object", not "obj". I understand this was just copied and
pasted, but still I would prefer a word to an abbreviation.

>      void postNotification(RenderObject*, AXNotification, bool postToElement, PostType = PostAsynchronously);
> +    void postNotification(AccessibilityObject*, AXNotification, bool postToElement, PostType = PostAsynchronously);

I think the new postNotification code should share code with the old one. Maybe
the old one could call the new one?

> +    return adoptRef(new AccessibilityScrollbar());

No parentheses needed here.

> +        return 0.0f;

Just 0 is our style, not 0.0f.

> +class AccessibilityScrollbar : public AccessibilityObject {
> +    
> +public:

No blank line needed.

> +    virtual ~AccessibilityScrollbar() { }

No need to define this destructor explicitly. It should work fine without this.

> +    virtual AccessibilityRole roleValue() const { return ScrollBarRole; }

Can’t you call setRoleValue instead of overriding this?

> +    virtual bool accessibilityIsIgnored() const { return false; }

I think this should be private since it would be a programming mistake to call
it directly.

> +    virtual IntSize size() const { return IntSize(); }
> +    virtual IntRect elementRect() const { return IntRect(); }
> +    virtual AccessibilityObject* parentObject() const { return 0; }
> +    virtual float valueForRange() const;

These could probably also all be private.

Is it really right for these all to be empty and zero in this way? Shouldn’t
the scrollbar be a child of the accessibility object representing the container
it scrolls or something?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list