[webkit-reviews] review canceled: [Bug 122104] [ATK] Expose state of aria-invalid as ATK_STATE_INVALID_ENTRY : [Attachment 213251] Patch proposal
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 4 02:44:16 PDT 2013
Mario Sanchez Prada <mario at webkit.org> has canceled Mario Sanchez Prada
<mario at webkit.org>'s request for review:
Bug 122104: [ATK] Expose state of aria-invalid as ATK_STATE_INVALID_ENTRY
https://bugs.webkit.org/show_bug.cgi?id=122104
Attachment 213251: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=213251&action=review
------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
(In reply to comment #4)
> (From update of attachment 213251 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=213251&action=review
>
> > Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:729
> > + if (!invalidStatus.isEmpty() && invalidStatus != "false")
>
> invalidStatus should never be empty based on previous patch you submitted
Argh... true. Changed it
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:1045
> > + // If no text attribute was found, just return what the state
says.
>
> this comment is not necessary. you're just explaining what the code does
rather than why it does it
>
Removed
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:1051
> > + if (isInvalidState != (attributeValue != "false"))
>
> this is a little confusing. maybe split this into to two if/if statements for
clarity?
I replaced it with this:
bool isAriaInvalid = attributeValue != "false";
if (isInvalidState != isAriaInvalid) {
return JSStringCreateWithCharacters(0, 0);
}
I guess that way is more clear now.
More information about the webkit-reviews
mailing list