Adds accessibility role for figcaption tag

Adds accessibility role for figcaption tag

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

[je_julie(Not used), 2014-09-03 10:02:51]

je_julie.kim@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[je_julie(Not used), 2014-09-03 10:03:12]

dmazzoni, PTAL.

[dmazzoni, 2014-09-03 21:19:23]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2014-09-03 21:19:23]

lgtm

[commit-bot: I haz the power, 2014-09-03 21:20:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/je_julie.kim@samsung.com/533893004/1

[dmazzoni, 2014-09-03 21:26:18]

https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
File Source/core/accessibility/AXRenderObject.cpp (right):

https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
Source/core/accessibility/AXRenderObject.cpp:584: bool
AXRenderObject::computeAccessibilityIsIgnored() const
Note: I just realized you may want to add support for these new roles you added
here.

In particular, I'm worried about a case like this:

<figcaption>
  <div>
    <div>
      Some text
    </div>
  </div>
</figcaption>

The logic here is designed to "ignore" nodes that don't seem to add anything
useful to the accessibility tree. It definitely eliminates multiple nested DIV
elements, for example. If you don't add support, it may eliminate the FIGCAPTION
element - but you should try it and see if that's an issue.

[commit-bot: I haz the power, 2014-09-03 22:50:56]

Committed patchset #1 (id:1) as 181349

[je_julie(Not used), 2014-09-04 04:55:19]

On 2014/09/03 21:26:18, dmazzoni wrote:
>
https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
> File Source/core/accessibility/AXRenderObject.cpp (right):
> 
>
https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
> Source/core/accessibility/AXRenderObject.cpp:584: bool
> AXRenderObject::computeAccessibilityIsIgnored() const
> Note: I just realized you may want to add support for these new roles you
added
> here.
> 
> In particular, I'm worried about a case like this:
> 
> <figcaption>
>   <div>
>     <div>
>       Some text
>     </div>
>   </div>
> </figcaption>
> 
> The logic here is designed to "ignore" nodes that don't seem to add anything
> useful to the accessibility tree. It definitely eliminates multiple nested DIV
> elements, for example. If you don't add support, it may eliminate the
FIGCAPTION
> element - but you should try it and see if that's an issue.

[je_julie(Not used), 2014-09-04 04:58:24]

On 2014/09/04 04:55:19, je_julie.kim wrote:
> On 2014/09/03 21:26:18, dmazzoni wrote:
> >
>
https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
> > File Source/core/accessibility/AXRenderObject.cpp (right):
> > 
> >
>
https://codereview.chromium.org/533893004/diff/1/Source/core/accessibility/AX...
> > Source/core/accessibility/AXRenderObject.cpp:584: bool
> > AXRenderObject::computeAccessibilityIsIgnored() const
> > Note: I just realized you may want to add support for these new roles you
> added
> > here.
> > 
> > In particular, I'm worried about a case like this:
> > 
> > <figcaption>
> >   <div>
> >     <div>
> >       Some text
> >     </div>
> >   </div>
> > </figcaption>
> > 
> > The logic here is designed to "ignore" nodes that don't seem to add anything
> > useful to the accessibility tree. It definitely eliminates multiple nested
DIV
> > elements, for example. If you don't add support, it may eliminate the
> FIGCAPTION
> > element - but you should try it and see if that's an issue.
Thank you for your information. I should check that case.
I'll look into it and update status soon.

[je_julie(Not used), 2014-09-04 08:26:11]

dmazzoni,
As you pointed out,
We miss figcaption info with that test case on the current code.
<figcaption>
  <div><div>
      Some text
  </div></div>
</figcaption>

  id=8 div READONLY (8, 135)-(1076, 18) value= display=block html_tag=div name=
child_ids=15
    id=15 staticText READONLY (8, 135)-(65, 17) value=Some text display=block
name= child_ids=16
      id=16 inlineTextBox (8, 135)-(65, 17) text_direction=lr value=Some text
name= character_offsets=9,17,29,36,40,45,52,60,64 word_starts=0,5 word_ends=4,9

After I added code to computeAccessibilityIsIgnored(), I have the result below.
  id=6 figcaption READONLY (8, 135)-(1076, 18) value= display=block
html_tag=figcaption name= child_ids=14
    id=14 div READONLY (8, 135)-(1076, 18) value= display=block html_tag=div
name= child_ids=15
      id=15 staticText READONLY (8, 135)-(65, 17) value=Some text display=block
name= child_ids=16
        id=16 inlineTextBox (8, 135)-(65, 17) text_direction=lr value=Some text
name= character_offsets=9,17,29,36,40,45,52,60,64 word_starts=0,5 word_ends=4,9

I think I should update code in AXRenderObject.cpp in order not to miss
figcaption info on any case.
In this case, can I upload my additional change here or should I create new
issue?

[dmazzoni, 2014-09-04 15:16:25]

This change was already committed, so please just make a new change. You can
handle both figure and figcaption in that change if needed.