[webkit-reviews] review denied: [Bug 121977] Regression: AX: <table><caption> no longer exposed as AXTitle. : [Attachment 213114] Patch.

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has denied Samuel White
<samuel_white at apple.com>'s request for review:
Bug 121977: Regression: AX: <table><caption> no longer exposed as AXTitle.
https://bugs.webkit.org/show_bug.cgi?id=121977

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

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=213114&action=review


r- to add the missing checks and to share more code between the two methods.

> Source/WebCore/accessibility/AccessibilityTable.cpp:555
> +void AccessibilityTable::titleElementText(Vector<AccessibilityText>&
textOrder) const
> +{

Seems like there are some checks missing here that appear in other methods,
namely:

if (!isAccessibilityTable())
    return;

if (!m_renderer)
    return

> Source/WebCore/accessibility/AccessibilityTable.cpp:561
> +    Node* node = this->node();
> +    if (node && isHTMLTableElement(node)) {
> +	   HTMLTableCaptionElement* caption =
toHTMLTableElement(node)->caption();
> +	   if (caption)
> +	       textOrder.append(AccessibilityText(caption->innerText(),
LabelByElementText));
> +    }

Instead of duplicating this code from AccessibilityTable::title(), try to share
the code in AccessibilityTable::title() with this method.

Also, shouldn't you use m_renderer->node() instead of this->node() as in
AccessibilityTable::title() below?