[webkit-reviews] review granted: [Bug 27964] WAI-ARIA: radio button does not determine its label from text content : [Attachment 34070] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 27964: WAI-ARIA: radio button does not determine its label from text
content
https://bugs.webkit.org/show_bug.cgi?id=27964

Attachment 34070: patch
https://bugs.webkit.org/attachment.cgi?id=34070&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -	       ariaLabel.append(' ');
> +	       
> +	       if (i != (size-1))
> +		   ariaLabel.append(' ');

Conventional format would be:

    if (i != size - 1)

without the extra parentheses and with spaces around the operator. Also, to
avoid overflow, I think it should be:

    if (i + 1 < size)

instead.

Generally speaking it seems that the textUnderElement ought to contain the same
kind of whitespace-collapsing logic that the normal rendering code does. This
could be done by using the TextIterator instead of walking the nodes and
concatenating them all. Not sure what bug it will cause that this just appends
the DOM, but I suspect it will cause problems, especially when there are 

Also, I noticed that ariaAccessiblityName misspells the word accessibility.

> +2009-08-04  Chris Fleizach  <cfleizach at apple.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Added test for 
> +	   Bug 27964 - WAI-ARIA: radio button does not determine its label from
text content
> +	   https://bugs.webkit.org/show_bug.cgi?id=27964
> +
> +	   Updated tests that expected the extra space at the end of some ARIA
labels.
> +
> +	   * accessibility/aria-labelledby-stay-within.html:
> +	   * platform/mac/accessibility/aria-describedby-on-input-expected.txt:

> +	   * platform/mac/accessibility/aria-labelledby-on-input-expected.txt:
> +	   * platform/mac/accessibility/aria-radiobutton-text-expected.txt:
Added.
> +	   * platform/mac/accessibility/aria-radiobutton-text.html: Added.
> +
> +2009-08-04  Chris Fleizach  <cfleizach at apple.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Need a short description and bug URL (OOPS!)
> +
> +	   * accessibility/aria-labelledby-stay-within.html:
> +	   * platform/mac/accessibility/aria-describedby-on-input-expected.txt:

> +	   * platform/mac/accessibility/aria-labelledby-on-input-expected.txt:
> +

Double change log here.

Otherwise looks fine.

r=me as is, but consider the improvements I suggested.