Ignore text elements for menu item related roles.

Ignore text elements for menu item related roles.

Text elements should go along with menu item related roles.
Let's consider this example: <div role="menuitemcheckbox" tabindex="0">menu item checkbox</div>
Currently MenuItemCheckBoxRole is represented by accessible object and its title is the second
accessible object. Expected behavior is that we should have just one accessible object.

BUG=442278

[k.czech, 2014-12-17 16:04:53]

k.czech@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[k.czech, 2014-12-17 16:07:27]

On 2014/12/17 16:04:54, k.czech wrote:

This is second attempt to land this patch. Previously win bots protested. Two
tests were failing.
Basically those expectations are wrong. I propose an update
https://codereview.chromium.org/807173005/.
I'm wondering how to synchronize those two patches to have win bots green ?.

[shreeramk, 2014-12-17 17:41:02]

On 2014/12/17 16:07:27, k.czech wrote:
> On 2014/12/17 16:04:54, k.czech wrote:
> 
> This is second attempt to land this patch. Previously win bots protested. Two
> tests were failing.
> Basically those expectations are wrong. I propose an update
> https://codereview.chromium.org/807173005/.
> I'm wondering how to synchronize those two patches to have win bots green ?.

1. You need to submit a chromium side patch disabling those two failing AX tests
in dump_accessibility_tree_test.cc
2. Submit your blink side change.
3. Submit a patch enabling and updating the AX test in chromium side

[k.czech, 2014-12-18 15:19:04]

On 2014/12/17 17:41:02, shreeramk wrote:
> On 2014/12/17 16:07:27, k.czech wrote:
> > On 2014/12/17 16:04:54, k.czech wrote:
> > 
> > This is second attempt to land this patch. Previously win bots protested.
Two
> > tests were failing.
> > Basically those expectations are wrong. I propose an update
> > https://codereview.chromium.org/807173005/.
> > I'm wondering how to synchronize those two patches to have win bots green ?.
> 
> 1. You need to submit a chromium side patch disabling those two failing AX
tests
> in dump_accessibility_tree_test.cc
> 2. Submit your blink side change.
> 3. Submit a patch enabling and updating the AX test in chromium side

Thanks for clarifying me this.

Provided patch(https://codereview.chromium.org/811413002/) to disable those
tests.

[k.czech, 2014-12-29 15:50:15]

Hi Dominic any chance to get review ?. I already disabled AX tests that were
failing (https://codereview.chromium.org/811413002/). I guess this one could go
in.

[dmazzoni, 2015-01-05 09:10:06]

Thanks for your patience - I'm back from vacation and catching up on reviews.

Can you explain why you'd like to make this change?

Just about everywhere else when an ARIA object has inner text, we expose the
accessible object for the inner text too. I'm not sure why we'd want menu items
to be any different.

One argument against the change you're proposing is that it adds a bunch of
checks to an already slow function. If anything I'm trying to figure out ways to
get rid of a lot of logic in computeAccessibilityIsIgnored so that it runs
quickly, even if that means slightly more nodes in the AX tree.

Is there a specific user-facing bug that motivated this change? If so, we should
investigate other ways to address it.

[k.czech, 2015-01-07 10:43:53]

On 2015/01/05 09:10:06, dmazzoni wrote:
> Thanks for your patience - I'm back from vacation and catching up on reviews.
> 
> Can you explain why you'd like to make this change?
> 
> Just about everywhere else when an ARIA object has inner text, we expose the
> accessible object for the inner text too. I'm not sure why we'd want menu
items
> to be any different.
> 
> One argument against the change you're proposing is that it adds a bunch of
> checks to an already slow function. If anything I'm trying to figure out ways
to
> get rid of a lot of logic in computeAccessibilityIsIgnored so that it runs
> quickly, even if that means slightly more nodes in the AX tree.
> 
> Is there a specific user-facing bug that motivated this change? If so, we
should
> investigate other ways to address it.
ad
Hi Dominic

Basically I was following the existing code. Static text beneath menu items are
already ignored.
I'm just adjusting code for other menu related items (chekboxes and radios).
Other reason was to get rid of the additional objects from AX tree.

[k.czech, 2015-01-07 11:03:26]

On 2015/01/07 10:43:53, k.czech wrote:
> On 2015/01/05 09:10:06, dmazzoni wrote:
> > Thanks for your patience - I'm back from vacation and catching up on
reviews.
> > 
> > Can you explain why you'd like to make this change?
> > 
> > Just about everywhere else when an ARIA object has inner text, we expose the
> > accessible object for the inner text too. I'm not sure why we'd want menu
> items
> > to be any different.
> > 
> > One argument against the change you're proposing is that it adds a bunch of
> > checks to an already slow function. If anything I'm trying to figure out
ways
> to
> > get rid of a lot of logic in computeAccessibilityIsIgnored so that it runs
> > quickly, even if that means slightly more nodes in the AX tree.
> > 
> > Is there a specific user-facing bug that motivated this change? If so, we
> should
> > investigate other ways to address it.
> ad
> Hi Dominic
> 
> Basically I was following the existing code. Static text beneath menu items
are
> already ignored.
> I'm just adjusting code for other menu related items (chekboxes and radios).
> Other reason was to get rid of the additional objects from AX tree.

You are right there are quite a bit of ifs behind the
computeAccessibilityIsIgnored and it might slow down in certain situations.
Basically I was also going to propose the same logic for anchors where the text
could be reported along with those elements.