Adding a11y support for HTML5 <datalist>

Adding a11y support for HTML5 <datalist>

BUG=282005
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186505

[ramya.v, 2014-11-27 12:00:05]

ramya.v@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[ramya.v, 2014-11-27 12:00:06]

Hi dmazzoni, can you please review this patch. Thanks

[ramya.v, 2014-11-27 12:00:50]

ramya.v@samsung.com changed reviewers:
+ aboxhall@chromium.org
- dmazzoni@chromium.org

[ramya.v, 2014-11-27 12:00:50]

Hi Can you please review this patch. Thanks

[ramya.v, 2014-11-27 12:04:10]

ramya.v@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[ramya.v, 2014-11-27 12:04:10]

Please review

[ramya.v, 2014-11-28 09:11:11]

On 2014/11/27 12:04:10, ramya.v wrote:
> Please review

@dmazzoni and aboxhall - Could you please review the patch. Thanks!

[dmazzoni, 2014-11-29 07:42:27]

Could you add support fo AXNodeObject too in a follow-up change?

As a reminder, that's used inside of a canvas, or inside aria-hidden=false, for
example:

<canvas>
  <datalist>...</datalist>
</canvas>

<div style="display:none" aria-hidden=false>
  <datalist>...</datalist>
</div>

Those are the two cases currently where we have a node that's part of the AX
tree but doesn't have a renderer.

Even better would be to try to refactor the role computation code so that
AXNodeObject and AXRenderObject share more of the same code.

[dmazzoni, 2014-11-29 07:42:41]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-11-29 07:43:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/762783002/1

[commit-bot: I haz the power, 2014-11-29 07:43:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-29 07:43:23]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[ramya.v, 2014-12-02 10:46:52]

On 2014/11/29 07:43:23, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.



@dmazzoni Thank you very much for the review comments.
Refactored the code.
PTAL

Thanks

[ramya.v, 2014-12-04 03:52:46]

On 2014/12/02 10:46:52, ramya.v wrote:
> On 2014/11/29 07:43:23, I haz the power (commit-bot) wrote:
> > No LGTM from a valid reviewer yet. Only full committers are accepted.
> > Even if an LGTM may have been provided, it was from a non-committer,
> > _not_ a full super star committer.
> > See http://www.chromium.org/getting-involved/become-a-committer
> > Note that this has nothing to do with OWNERS files.
> 
> 
> 
> @dmazzoni Thank you very much for the review comments.
> Refactored the code.
> PTAL
> 
> Thanks

@dmazzoni Could you please review the patch.

Thanks

[dmazzoni, 2014-12-04 08:23:20]

lgtm

Thanks for pulling out some of the common code!

https://codereview.chromium.org/762783002/diff/80001/Source/modules/accessibi...
File Source/modules/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/762783002/diff/80001/Source/modules/accessibi...
Source/modules/accessibility/AXNodeObject.cpp:179: AccessibilityRole
AXNodeObject::determineAccessibilityRoleUtil()
Need a newline before this function.

[ramya.v, 2014-12-04 09:33:10]

On 2014/12/04 08:23:20, dmazzoni wrote:
> lgtm
> 
> Thanks for pulling out some of the common code!
> 
>
https://codereview.chromium.org/762783002/diff/80001/Source/modules/accessibi...
> File Source/modules/accessibility/AXNodeObject.cpp (right):
> 
>
https://codereview.chromium.org/762783002/diff/80001/Source/modules/accessibi...
> Source/modules/accessibility/AXNodeObject.cpp:179: AccessibilityRole
> AXNodeObject::determineAccessibilityRoleUtil()
> Need a newline before this function.

Added newline before determineAccessibilityRoleUtil function in patchset 6
PTAL.

[ramya.v, 2014-12-04 09:44:26]

The CQ bit was checked by ramya.v@samsung.com

[commit-bot: I haz the power, 2014-12-04 09:45:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/762783002/100001

[commit-bot: I haz the power, 2014-12-04 10:50:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-04 10:50:17]

Failed to apply patch for Source/modules/accessibility/AXRenderObject.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/modules/accessibility/AXRenderObject.cpp
  Hunk #5 FAILED at 343.
  Hunk #6 succeeded at 366 (offset 4 lines).
  1 out of 6 hunks FAILED -- saving rejects to file
Source/modules/accessibility/AXRenderObject.cpp.rej

Patch:       Source/modules/accessibility/AXRenderObject.cpp
Index: Source/modules/accessibility/AXRenderObject.cpp
diff --git a/Source/modules/accessibility/AXRenderObject.cpp
b/Source/modules/accessibility/AXRenderObject.cpp
index
536ca17f0c75551e8cc506925161ac76ebe04a01..33a10e905929472f784dc51879f8a540619da5f3
100644
--- a/Source/modules/accessibility/AXRenderObject.cpp
+++ b/Source/modules/accessibility/AXRenderObject.cpp
@@ -264,36 +264,23 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     Node* node = m_renderer->node();
     RenderBoxModelObject* cssBox = renderBoxModelObject();
 
-    if (node && node->isLink()) {
-        if (cssBox && cssBox->isImage())
-            return ImageMapRole;
-        return LinkRole;
-    }
     if ((cssBox && cssBox->isListItem()) || isHTMLLIElement(node))
         return ListItemRole;
     if (m_renderer->isListMarker())
         return ListMarkerRole;
-    if (isHTMLButtonElement(node))
-        return buttonRoleType();
-    if (isHTMLDetailsElement(node))
-        return DetailsRole;
-    if (isHTMLSummaryElement(node)) {
-        if (node->parentElement() &&
isHTMLDetailsElement(node->parentElement()))
-            return DisclosureTriangleRole;
-        return UnknownRole;
-    }
     if (isHTMLLegendElement(node))
         return LegendRole;
     if (m_renderer->isText())
         return StaticTextRole;
     if (cssBox && cssBox->isImage()) {
+        if (node && node->isLink())
+            return ImageMapRole;
         if (isHTMLInputElement(node))
             return ariaHasPopup() ? PopUpButtonRole : ButtonRole;
         if (isSVGImage())
             return SVGRootRole;
         return ImageRole;
     }
-
     // Note: if JavaScript is disabled, the renderer won't be a
RenderHTMLCanvas.
     if (isHTMLCanvasElement(node) && m_renderer->isCanvas())
         return CanvasRole;
@@ -301,53 +288,6 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     if (cssBox && cssBox->isRenderView())
         return WebAreaRole;
 
-    if (cssBox && cssBox->isTextArea())
-        return TextAreaRole;
-
-    if (isHTMLInputElement(node)) {
-        HTMLInputElement& input = toHTMLInputElement(*node);
-        const AtomicString& type = input.type();
-        if (type == InputTypeNames::button) {
-            if ((node->parentNode() && isHTMLMenuElement(node->parentNode()))
|| (parentObject() && parentObject()->roleValue() == MenuRole))
-                return MenuItemRole;
-            return buttonRoleType();
-        }
-        if (type == InputTypeNames::checkbox) {
-            if ((node->parentNode() && isHTMLMenuElement(node->parentNode()))
|| (parentObject() && parentObject()->roleValue() == MenuRole))
-                return MenuItemCheckBoxRole;
-            return CheckBoxRole;
-        }
-        if (type == InputTypeNames::date)
-            return DateRole;
-        if (type == InputTypeNames::datetime
-            || type == InputTypeNames::datetime_local
-            || type == InputTypeNames::month
-            || type == InputTypeNames::week)
-            return DateTimeRole;
-        if (type == InputTypeNames::radio) {
-            if ((node->parentNode() && isHTMLMenuElement(node->parentNode()))
|| (parentObject() && parentObject()->roleValue() == MenuRole))
-                return MenuItemRadioRole;
-            return RadioButtonRole;
-        }
-        if (type == InputTypeNames::file)
-            return ButtonRole;
-        if (type == InputTypeNames::number)
-            return SpinButtonRole;
-        if (input.isTextButton())
-            return buttonRoleType();
-        if (type == InputTypeNames::color)
-            return ColorWellRole;
-        if (type == InputTypeNames::time)
-            return TimeRole;
-        return TextFieldRole;
-    }
-
-    if (cssBox && cssBox->isMenuList())
-        return PopUpButtonRole;
-
-    if (headingLevel())
-        return HeadingRole;
-
     if (m_renderer->isSVGImage())
         return ImageRole;
     if (m_renderer->isSVGRoot())
@@ -356,9 +296,6 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     if (node && node->hasTagName(ddTag))
         return DescriptionListDetailRole;
 
-    if (node && node->hasTagName(dlTag))
-        return DescriptionListRole;
-
     if (node && node->hasTagName(dtTag))
         return DescriptionListTermRole;
 
@@ -375,33 +312,12 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     if (m_renderer->isHR())
         return SplitterRole;
 
-    if (isHTMLOutputElement(node))
-        return StatusRole;
-
-    if (isHTMLParagraphElement(node))
-        return ParagraphRole;
-
-    if (isHTMLLabelElement(node))
-        return LabelRole;
-
-    if (isHTMLRubyElement(node))
-        return RubyRole;
-
-    if (isHTMLDivElement(node))
-        return DivRole;
-
-    if (isHTMLMeterElement(node))
-        return MeterRole;
-
     if (isHTMLFormElement(node))
         return FormRole;
 
     if (node && node->hasTagName(articleTag))
         return ArticleRole;
 
-    if (node && node->hasTagName(blockquoteTag))
-        return BlockquoteRole;
-
     if (node && node->hasTagName(mainTag))
         return MainRole;
 
@@ -427,17 +343,6 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     if (isHTMLHtmlElement(node))
         return IgnoredRole;
 
-    if (node && node->hasTagName(iframeTag))
-        return IframeRole;
-
-    if (isEmbeddedObject())
-        return EmbeddedObjectRole;
-
-    if (node && node->hasTagName(figcaptionTag))
-        return FigcaptionRole;
-
-    if (node && node->hasTagName(figureTag))
-        return FigureRole;
 
     // There should only be one banner/contentInfo per page. If header/footer
are being used within an article or section
     // then it should not be exposed as whole page's banner/contentInfo
@@ -446,8 +351,9 @@ AccessibilityRole
AXRenderObject::determineAccessibilityRole()
     if (node && node->hasTagName(footerTag) &&
!isDescendantOfElementType(articleTag) &&
!isDescendantOfElementType(sectionTag))
         return FooterRole;
 
-    if (isHTMLAnchorElement(node) && isClickable())
-        return LinkRole;
+    AccessibilityRole role = AXNodeObject::determineAccessibilityRoleUtil();
+    if (role != UnknownRole)
+        return role;
 
     if (m_renderer->isRenderBlockFlow())
         return GroupRole;

[commit-bot: I haz the power, 2014-12-04 10:50:21]

The CQ bit was unchecked by commit-bot@chromium.org

[ramya.v, 2014-12-04 13:21:43]

The CQ bit was checked by ramya.v@samsung.com

[commit-bot: I haz the power, 2014-12-04 13:22:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/762783002/120001

[commit-bot: I haz the power, 2014-12-04 13:33:58]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=186505