[webkit-reviews] review canceled: [Bug 84137] [GTK] Properly expose <legend> elements to ATs : [Attachment 161212] proposed patch

[bugzilla-daemon at webkit.org]

Joanmarie Diggs (irc: joanie) <jdiggs at igalia.com> has canceled Joanmarie Diggs
(irc: joanie) <jdiggs at igalia.com>'s request for review:
Bug 84137: [GTK] Properly expose <legend> elements to ATs
https://bugs.webkit.org/show_bug.cgi?id=84137

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

------- Additional Comments from Joanmarie Diggs (irc: joanie)
<jdiggs at igalia.com>
(In reply to comment #12)
> (From update of attachment 161212 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=161212&action=review
> 
> > LayoutTests/accessibility/legend.html:52
> > +	     buildAccessibilityTree(body, 0, fieldset.titleUIElement(), "<<
fieldset's titleUIElement");
> 
> While this method works, I find it a little hard to understand. I think in
addition to what you have here, doing an equality for the titleUIElement would
be good.
> 
> something like
> 
> shouldBeTrue(titleUIElement.isEqual(fieldset.titleUIElement()))
> 
> after you build the accessibility tree code.

Wound up doing three things in response:

1. Null check:

   shouldBeTrue("titleUIElement != null");

2. Platform independent (in theory) search for the text:

   var titleUIElementText = titleUIElement;
   while (titleUIElementText && titleUIElementText.childrenCount)
       titleUIElementText = titleUIElementText.childAtIndex(0); 
   shouldBe("titleUIElementText.stringValue", "'AXValue: Choose a shipping
method:'");

   ("In theory" == looking at the html and how we do things, the ancestry will
vary
    from platform to platform, but I can't think of a good reason why a
platform would
    give the StaticText object a child. Moreover, I confirmed it works for the
Gtk port
    as well as the Mac port.)

3. Added comments as to why the test was doing the things it is doing.

In addition, I updated the Gtk test results to reflect what will be the case
after the fix for bug 95180 is committed since you gave that one an R+ and I've
since Cq?ed it.
 
> > Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:205
> > +	 
> 
> extra line insertion can be removed

Argh. I hate when I do that. Thanks for catching it. Removed.