[Webkit-unassigned] [Bug 37275] Accessibility support for progress element

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #52872|review?                     |review-
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2010-04-08 12:25:34 PST ---
(From update of attachment 52872)
> +bool AccessibilityProgressIndicator::accessibilityIsIgnored() const
> +{
> +    AccessibilityObjectInclusion decision = accessibilityIsIgnoredBase();
> +    if (decision == IncludeObject)
> +        return false;
> +    if (decision == IgnoreObject)
> +        return true;
> +    
> +    return false;
> +}

How about writing it like this instead?

    return accessibilityIsIgnoredBase() == IgnoreObject;

> +    // Indneterminate progress bar should return 0.

Typo: "Indneterminate"

> +HTMLProgressElement* AccessibilityProgressIndicator::element() const
> +{
> +    return static_cast<HTMLProgressElement*>(m_renderer->node());
> +}

I don't like having a risky typecast here so far away from the code that
guarantees that the type is an HTMLProgressElement. Ideally the renderer itself
would have a function that returns an HTMLProgressElement* or we would put both
an assertion and a runtime check here.

> +    virtual ~AccessibilityProgressIndicator() { }

Should just drop this. It adds nothing.

> +    virtual AccessibilityRole roleValue() const { return ProgressIndicatorRole; }
> +
> +    virtual bool isProgressIndicator() const { return true; }
> +
> +    virtual float valueForRange() const;
> +    virtual float maxValueForRange() const;
> +    virtual float minValueForRange() const;

These member functions should all be private.

> +protected:
> +    AccessibilityProgressIndicator(RenderObject*);

This too.

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