467139 - NameFromSubtree rule should be based on role

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

This is the part of the bug 455886.

Here ARIA nameFromSubtree rules will be dropped and all explicit calls of GetHTML/XULName that are used to point whether we should get the name from subtree or not will be dropped also.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[[e-mail]]

Comment on attachment 350556 [details] [diff] [review]
patch

I was just curious as to how moving this flag to a separate array improves the code.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> (From update of attachment 350556 [details] [diff] [review])
> I was just curious as to how moving this flag to a separate array improves the
> code.

In the meantime this flag is applied for ARIA accessibles only. I extend its usage to all accessibles.

Comment 4

[Marco Zehe (:MarcoZ)]

Comment on attachment 350556 [details] [diff] [review]
patch

Tests pass, tree comparison of google.com, mozilla.org and my blog don't show any difference, and while surfing with a build containing this patch, also didn't find any difference so far. r=me.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 350556 [details] [diff] [review]
patch

Nice work. I agree the name finding shouldn't be ARIA specific. Regarding the removal of the GetNameInternal overrides, I think this is logically correct, but I'm unsure of the perf cost since the base class does some extra work.

Do we have profile tests?

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> (From update of attachment 350556 [details] [diff] [review])
> Nice work. I agree the name finding shouldn't be ARIA specific. Regarding the
> removal of the GetNameInternal overrides, I think this is logically correct,
> but I'm unsure of the perf cost since the base class does some extra work.

David, what kind of extra work? Do you mean checking the HTML namespace? That's the difference between what was and what is.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)

> David, what kind of extra work? Do you mean checking the HTML namespace?

And sometimes XUL of course.

Comment 8

[Aaron Leventhal]

David, the perf hit of getting the role is pretty low. This patch is fine.

Comment 9

[Aaron Leventhal]

(In reply to comment #4)
> (From update of attachment 350556 [details] [diff] [review])
> Tests pass, tree comparison of google.com, mozilla.org and my blog don't show
> any difference, and while surfing with a build containing this patch, also
> didn't find any difference so far. r=me.

Did you do your tree comparison with Speclenium?
What about the suite of ARIA tests the Eitan put into that?

The patch looks harmless, but I want to be sure we keep ARIA right. It would be good to check out the prefs dialog a bit as well.

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> (In reply to comment #4)
> > (From update of attachment 350556 [details] [diff] [review] [details])
> > Tests pass, tree comparison of google.com, mozilla.org and my blog don't show
> > any difference, and while surfing with a build containing this patch, also
> > didn't find any difference so far. r=me.
> 
> Did you do your tree comparison with Speclenium?

I'm sure Marco did this exactly.

Comment 11

[Aaron Leventhal]

(In reply to comment #2)
> (From update of attachment 350556 [details] [diff] [review])
> I was just curious as to how moving this flag to a separate array improves the
> code.

Neil, it will allow us to fix the dependent bug, where we should not walk into container widgets when using name from subtree rule. We don't want it to walk into any container widget in the subtree, whether ARIA or not.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #8)
> David, the perf hit of getting the role is pretty low. This patch is fine.

Yup ok.

Comment 13

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/mozilla-central/rev/45104f550b52