Bug 98167 – AX:When aria-label is used, the text under an element is still appearing as the AXTitle

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

The WAI-ARIA text computation states that if aria-label or aria-labelledby is used, then we should not be exposing the text under the element as its accessible name
http://www.w3.org/TR/wai-aria/roles#textalternativecomputation

However, we are exposing this information in the title() which causes VoiceOver to speak undesired things.

For example
<a href="#" aria-label="LINK">test</a>

Will expose

AXTitle: test
AXDescription: LINK

Whereas the correct behavior would be to

AXTitle: 
AXDescription: LINK

[chris fleizach]

rdar://12379822

[Dominic Mazzoni]

It'd be great if we could comply with the text alternative computation.

Note that there's also this spec:
http://www.w3.org/TR/2012/WD-html-aapi-20120329/#calc

They're not mutually inconsistent, but they each contain slightly different details. Ideally we should support all of the suggestions of both.

My impression is that currently there's lots of historic logic in AccessibilityObject's title and description calculation that's there for historic reasons, or encodes platform-specific logic. For example:

* The help text (value of the "title" attribute) becomes the description if there's no description
* aria-labelledby and aria-describedby are supposed to be complementary, but they both affect the description
* Different behavior when an element's label is visible (titleUIElement) or hidden (it goes right into the description) - this interacts poorly with the logic that the help text becomes the description if the description is empty in the case where an element has both title and label.

Rather than trying to fix it incrementally and run the risk of regressions or lots of test churn, how about:

1. Implement new methods: textAlternativeName(), textAlternativeDescription(), and textAlternativeHelp() - these would implement the text alternative computations defined by the specs, starting from scratch with a clean, platform-independent implementation. Add support for testing these in a cross-platform way (add attributes to the accessible wrapper only to be used by DRT/WKTR). We could do this in a series of changes, no production code would be affected.

2. Rewrite most cross-platform tests that access the title or description (anything in LayoutTests/accessibility) to use these new methods.

3. Then deprecate AccessibilityObject::title() and AccessibilityObject::description() and reimplement the platform-specific text methods (AXTitle / AXDescription for Mac, etc.) in terms of these new cross-platform methods, and update the tests. This could be done separately for each platform, rather than trying to coordinate them all at once.

4. Finally, get rid of title() and description(), etc. from AccessibilityObject.

[chris fleizach]

(In reply to comment #2)
> It'd be great if we could comply with the text alternative computation.
> 
> Note that there's also this spec:
> http://www.w3.org/TR/2012/WD-html-aapi-20120329/#calc
> 
> They're not mutually inconsistent, but they each contain slightly different details. Ideally we should support all of the suggestions of both.

In this instance they are mutually exclusive. If a link has an aria-label, we shouldn't be exposing what's on the screen.

> 
> My impression is that currently there's lots of historic logic in AccessibilityObject's title and description calculation that's there for historic reasons, or encodes platform-specific logic. For example:
> 
> * The help text (value of the "title" attribute) becomes the description if there's no description

This is actually part of the WAI-ARIA text computation algorithm

> * aria-labelledby and aria-describedby are supposed to be complementary, but they both affect the description

This is a tough one because there's no good place for aria-describedby without another field. Hence labelledby takes precedence, but really they should both be exposed

> * Different behavior when an element's label is visible (titleUIElement) or hidden (it goes right into the description) - this interacts poorly with the logic that the help text becomes the description if the description is empty in the case where an element has both title and label.
> 
> Rather than trying to fix it incrementally and run the risk of regressions or lots of test churn, how about:
> 
> 1. Implement new methods: textAlternativeName(), textAlternativeDescription(), and textAlternativeHelp() - 

This is essentially what title(), accessibilityDescription(), and helpText() are, but your naming is better.

The problem that I've had is what is the difference between name()/title() and description(). On the mac, the difference should be that title() is what's visible. description is an alternative name. Due to that, we have this mishmash between title and description

This is further complicated by the WAI-ARIA computation which specifies basically this order

1) aria-label*
2) visible text
3) alt text
4) text under element
5) title attribute

So even if we separate them between visible and non-visible, we can't return some of the values if other values are present (i.e. the bug described in this bug)


Then the platforms could decide how to mix and 
these would implement the text alternative computations defined by the specs, starting from scratch with a clean, platform-independent implementation. Add support for testing these in a cross-platform way (add attributes to the accessible wrapper only to be used by DRT/WKTR). We could do this in a series of changes, no production code would be affected.
> 
> 2. Rewrite most cross-platform tests that access the title or description (anything in LayoutTests/accessibility) to use these new methods.
> 
> 3. Then deprecate AccessibilityObject::title() and AccessibilityObject::description() and reimplement the platform-specific text methods (AXTitle / AXDescription for Mac, etc.) in terms of these new cross-platform methods, and update the tests. This could be done separately for each platform, rather than trying to coordinate them all at once.
> 
> 4. Finally, get rid of title() and description(), etc. from AccessibilityObject.

[Dominic Mazzoni]

Thanks, I think I better understand the Mac attributes.

How about this idea:

* Compute a prioritized list of text associated with an accessible object, and with each one include its source, indicating whether it's visible or not. The prioritization order would be identical on all platforms and would totally respect the alternative name calculation spec - but the mapping to various native attributes could vary. *

For example:

<a href="#" aria-label="LINK">test</a>

Returns this list:
[ { text = "LINK", source = alternative },
  { text = "test", source = visible } ]

Or for another example:

<label>
  Address
  <input placeholder="123 Main St" title="Street address only">
</label>

Returns this list:
[ { text = "Address", source = visible },
  { text = "123 Main St", source = alternative },
  { text = "Street address only", source = alternative } ]

On Mac, the logic would be: If the first item in the list is visible, it becomes the title. Anything else in the list gets mapped to the description and help. If the first item in the list is not visible, the title is empty and the first item gets mapped to the description, and the next item gets mapped to the help.

On Windows, the logic is simpler: the first three items in the list get mapped to name, description, and help in that order.

The only thing I'm not sure how to handle is titleUIElement.

How does VoiceOver handle titleUIElement? Specifically, what does VoiceOver do if there's a titleUIElement and also a title? What about a titleUIElement and also a description?

Perhaps the list could just link directly to the object, if one exists and if it's visible. This would be handy because on Windows there's an equivalent of titleUIElement for description - but on Mac there's only one for title.

So I'm imagining something like this:

enum AccessibleTextSource {
    VISIBLE,
    ALTERNATIVE
};

struct AccessibleText {
    String text;
    AccessibilityObject* textObject; // Only 
    AccessibleTextSource source;
}

class AccessibilityObject {
    ...
    void accessibleText(Vector<AccessibleText>&);
}

I made the source an enum rather than a bool because perhaps there are more platform differences. Maybe the source should include INNER_TEXT, PLACEHOLDER, IMG_ALT, LABEL_ELEMENT, ... - which would give platforms lots of flexibility. For example, suppose that on one platform there's an explicit way to expose placeholder - that would give us an easy way to skip over the placeholder but still use the same prioritized list.

[chris fleizach]

(In reply to comment #4)
> Thanks, I think I better understand the Mac attributes.
> 
> How about this idea:
> 
> * Compute a prioritized list of text associated with an accessible object, and with each one include its source, indicating whether it's visible or not. The prioritization order would be identical on all platforms and would totally respect the alternative name calculation spec - but the mapping to various native attributes could vary. *
> 
> For example:
> 
> <a href="#" aria-label="LINK">test</a>
> 
> Returns this list:
> [ { text = "LINK", source = alternative },
>   { text = "test", source = visible } ]
> 
> Or for another example:
> 
> <label>
>   Address
>   <input placeholder="123 Main St" title="Street address only">
> </label>
> 
> Returns this list:
> [ { text = "Address", source = visible },
>   { text = "123 Main St", source = alternative },
>   { text = "Street address only", source = alternative } ]
> 

It gets more complicated. Placeholder is its own attribute on the Mac platform, so we want treat that separately sometimes.

> On Mac, the logic would be: If the first item in the list is visible, it becomes the title. Anything else in the list gets mapped to the description and help. If the first item in the list is not visible, the title is empty and the first item gets mapped to the description, and the next item gets mapped to the help.
> 

Another potential issue is dealing with help text. Does that get returned with this struct?

I think we might need to think of all the perverse cases before hand to ensure this system will work, but it seems doable.

> On Windows, the logic is simpler: the first three items in the list get mapped to name, description, and help in that order.
> 
> The only thing I'm not sure how to handle is titleUIElement.
> 
> How does VoiceOver handle titleUIElement? Specifically, what does VoiceOver do if there's a titleUIElement and also a title? What about a titleUIElement and also a description?
> 

I think VO will speak a title and a title ui element if both are present, but that might change based on the element type as well.

It would be nice to remove the logic in titleUIElement() and title() that depend on each other and move that to platform logic.

> Perhaps the list could just link directly to the object, if one exists and if it's visible. This would be handy because on Windows there's an equivalent of titleUIElement for description - but on Mac there's only one for title.
> 
> So I'm imagining something like this:
> 
> enum AccessibleTextSource {
>     VISIBLE,
>     ALTERNATIVE
> };
> 
> struct AccessibleText {
>     String text;
>     AccessibilityObject* textObject; // Only 
>     AccessibleTextSource source;
> }
> 
> class AccessibilityObject {
>     ...
>     void accessibleText(Vector<AccessibleText>&);
> }
> 
> I made the source an enum rather than a bool because perhaps there are more platform differences. Maybe the source should include INNER_TEXT, PLACEHOLDER, IMG_ALT, LABEL_ELEMENT, ... - which would give platforms lots of flexibility. For example, suppose that on one platform there's an explicit way to expose placeholder - that would give us an easy way to skip over the placeholder but still use the same prioritized list.

Yea I think an enum would be better here as well.

[Dominic Mazzoni]

(In reply to comment #5)
> It gets more complicated. Placeholder is its own attribute on the Mac platform, so we want treat that separately sometimes.

If we go with the enum idea this might work well. On Mac, it could just skip over placeholder while going through the list.

> Another potential issue is dealing with help text. Does that get returned with this struct?

Yes - but we could use the source enum to indicate the help text so a platform could ignore it.

> I think we might need to think of all the perverse cases before hand to ensure this system will work, but it seems doable.

Agreed. I still think we should implement and test the cross-platform part first.

[chris fleizach]

Created attachment 167271 [details]
potential solution

Here's what I was thinking. (This mostly works on the Mac, I only have 16/268 tests failing)

[Dominic Mazzoni]

Comment on attachment 167271 [details]
potential solution

View in context: https://bugs.webkit.org/attachment.cgi?id=167271&action=review

Some initial thoughts.

> Source/WebCore/accessibility/AccessibilityObject.h:214
> +struct AccessibilityText {

What do you think about including a reference to an AccessibilityObject here, similar to titleUIElement?

On win & atk, there's something equivalent to titleUIElement for both labeled-by and described-by. That would make titleUIElement easy to implement on top of accessibleText.

> Source/WebCore/accessibility/AccessibilityImageMapLink.cpp:106
> +        textOrder.append(AccessibilityText(titleText, AlternativeText));

should be TitleTagText

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1094
> +        if (webAreaText.isEmpty())

If not empty

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1097
> +    else {

Put return inside if block, get rid of else

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1098
> +        String ariaDescription = ariaAccessibilityDescription();

ariaAccessibilityDescription is confusing because it returns aria-label or aria-labelledby, not aria-describedby...

I'd like ariaLabeledBy to return the reference to the element, similar to titleUIElement. Doesn't have to be done in this change, but that suggests that ariaAccessibilityDescription should maybe be folded into this method.

[chris fleizach]

(In reply to comment #8)
> (From update of attachment 167271 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=167271&action=review
> 
> Some initial thoughts.
> 
> > Source/WebCore/accessibility/AccessibilityObject.h:214
> > +struct AccessibilityText {
> 
> What do you think about including a reference to an AccessibilityObject here, similar to titleUIElement?
> 
> On win & atk, there's something equivalent to titleUIElement for both labeled-by and described-by. That would make titleUIElement easy to implement on top of accessibleText.
> 

I felt like titleUIElement() wasn't as necessary since there's an existing method which is fairly straightforward. So the platform could always look at that method and make further calculations. Then the text calculation wouldn't have to worry about mixing elements and text with AccessibilityText

Can you describe more what

"On win & atk, there's something equivalent to titleUIElement for both labeled-by and described-by."

means

> > Source/WebCore/accessibility/AccessibilityImageMapLink.cpp:106
> > +        textOrder.append(AccessibilityText(titleText, AlternativeText));
> 
> should be TitleTagText
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1094
> > +        if (webAreaText.isEmpty())
> 
> If not empty
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1097
> > +    else {
> 
> Put return inside if block, get rid of else
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1098
> > +        String ariaDescription = ariaAccessibilityDescription();
> 
> ariaAccessibilityDescription is confusing because it returns aria-label or aria-labelledby, not aria-describedby...
> 
> I'd like ariaLabeledBy to return the reference to the element, similar to titleUIElement. Doesn't have to be done in this change, but that suggests that ariaAccessibilityDescription should maybe be folded into this method.

Right now labelled by just returns the text within the aria-labelledby attribute right? So would that buttress your desire to have an element within AccessibilityText? It almost seems like titleUIElement() and aria-labelledby are perhaps the same thing, and maybe titleUIElement() should first check labelled-by before other things

[Dominic Mazzoni]

(In reply to comment #9)
> I felt like titleUIElement() wasn't as necessary since there's an existing method which is fairly straightforward.

We can leave it there, but make it a shortcut that just iterates over accessibleText and returns the linked element for the first item in the list.

> Can you describe more what
> 
> "On win & atk, there's something equivalent to titleUIElement for both labeled-by and described-by."

For a native accessibility node, one of the APIs is to get all of the "relations" between that node and another node. That is supposed to include both nodes that "label" another element, and nodes that "describe" another element.

http://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/group__grp_relations.html

I think the "label" relation is the equivalent of titleUIElement, but we need an equivalent for the "described-by" relation. That make sense?

My idea that at the time when we see a node has either an aria-labelledby, aria-describedby, or an associated label element, we should add both the text and the link to the other element to the accessibleText array once, rather than duplicating that code. Then titleUIElement is just a convenience function.

> Right now labelled by just returns the text within the aria-labelledby attribute right? So would that buttress your desire to have an element within AccessibilityText? It almost seems like titleUIElement() and aria-labelledby are perhaps the same thing, and maybe titleUIElement() should first check labelled-by before other things

I think of aria-labelledby as the complement to label, so for that reason titleUIElement should return the node pointed to by aria-labelledby just as it would for the label element.

Aria-describedby is similar. On Mac, it just turns into description text, but on other platforms we have the option of returning the UI element it points to instead.

- Dominic

[Dominic Mazzoni]

Only one other general comment: it'd be great if we could test the new accessibilityText method directly from a layout test in a cross-platform manner, rather than having tests only get the platform-specific title, etc.

This is going to require multiple steps to complete. It might be better to get this to pass all tests now and check it in sooner, before changing any behavior. In other words, don't try to actually fix this bug ("When aria-label is used...") - just refactor and achieve the same behavior we have currently.

Would that help in getting the 16 tests that fail now to pass?

[chris fleizach]

(In reply to comment #10)
> (In reply to comment #9)
> > I felt like titleUIElement() wasn't as necessary since there's an existing method which is fairly straightforward.
> 
> We can leave it there, but make it a shortcut that just iterates over accessibleText and returns the linked element for the first item in the list.
> 
> > Can you describe more what
> > 
> > "On win & atk, there's something equivalent to titleUIElement for both labeled-by and described-by."
> 
> For a native accessibility node, one of the APIs is to get all of the "relations" between that node and another node. That is supposed to include both nodes that "label" another element, and nodes that "describe" another element.
> 
> http://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/group__grp_relations.html
> 
> I think the "label" relation is the equivalent of titleUIElement, but we need an equivalent for the "described-by" relation. That make sense?
> 
> My idea that at the time when we see a node has either an aria-labelledby, aria-describedby, or an associated label element, we should add both the text and the link to the other element to the accessibleText array once, rather than duplicating that code. Then titleUIElement is just a convenience function.
> 

Ok, I'm fine with that approach. I can see how it could be used in a cross-platform way.

> > Right now labelled by just returns the text within the aria-labelledby attribute right? So would that buttress your desire to have an element within AccessibilityText? It almost seems like titleUIElement() and aria-labelledby are perhaps the same thing, and maybe titleUIElement() should first check labelled-by before other things
> 
> I think of aria-labelledby as the complement to label, so for that reason titleUIElement should return the node pointed to by aria-labelledby just as it would for the label element.
> 

That sounds like the right change and inline with what I was thinking.

> Aria-describedby is similar. On Mac, it just turns into description text, but on other platforms we have the option of returning the UI element it points to instead.

Makes sense

> 
> - Dominic

[chris fleizach]

(In reply to comment #11)
> Only one other general comment: it'd be great if we could test the new accessibilityText method directly from a layout test in a cross-platform manner, rather than having tests only get the platform-specific title, etc.

Absolutely. Maybe that can come in the next patch in this sequence.

> 
> This is going to require multiple steps to complete. It might be better to get this to pass all tests now and check it in sooner, before changing any behavior. In other words, don't try to actually fix this bug ("When aria-label is used...") - just refactor and achieve the same behavior we have currently.
> 

I agree with that sentiment. 

> Would that help in getting the 16 tests that fail now to pass?

THose 16 tests are probably just minor bugs. you already found a few in your first review. Now that we think the approach is good, I can resolve those pretty quick

[Dominic Mazzoni]

Comment on attachment 167271 [details]
potential solution

View in context: https://bugs.webkit.org/attachment.cgi?id=167271&action=review

Took another full pass, here are a few more detailed comments.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1822
> +// If there's alternative text, that can override the title.

The first time I read this, it sounded like you could return something here but VoiceOver would let the description override - but actually this method needs to return nothing if there's alt text. To be more clear, maybe say "if there's alternative text that's meant to override the visible text, this will return the empty string so the alternative text can override the visible text."

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1830
> +        AccessibilityText text = textOrder[k];

const AccessibilityText& text = textOrder[k]; - and similarly in every other method, so you don't do an unnecessary copy.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1845
> +- (NSString *)accessibilityDescription

How about a complementary comment here, indicating this is gathered from alternative text.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1854
> +        // If we encounter alternative text, then do not expose the title.

Comment is wrong

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1863
> +        if (text.textSource == TitleTagText)

Just curious, why does "title" get promoted to description but not "summary"?

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1891
> +    return [NSString string];

I wonder if it might be more readable to combine these into one method that takes one pass over the string array and computes all three at once:

accessibilityTextMac(const Vector<AccessibilityText>& textOrder, String* title, String* description, String* help)
{
    *title = 0;
    *description = 0;
    *help = 0;

    unsigned length = textOrder.size();
    for (unsigned k = 0; k < length; k++) {
        const AccessibilityText& text = textOrder[k];

        if (text.textSource == VisibleText && !description)
            title = &text.text;
        else if (text.textSource == AlternativeText)
            description = &text.text;
        ...
    }
}

It would only update pointers when it finds each one, so it's not actually not less efficient than what you have. And it'd keep all of the logic in one place rather than the logic of accessibilityDescription heavily depending on the logic of accessibilityTitle and so on.

Just an idea. If that looks uglier then I'll keep thinking about how else we could avoid duplicating the logic.

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1138
> +        const AtomicString& ariaLabel = documentElement->getAttribute(aria_labelAttr);

Why not call ariaAccessibilityDescription here?

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1231
> +    String describedBy = ariaDescribedByAttribute();

I think aria-describedby should come ahead of aria-help.

[chris fleizach]

(In reply to comment #14)
> (From update of attachment 167271 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=167271&action=review
> 
> Took another full pass, here are a few more detailed comments.
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1822
> > +// If there's alternative text, that can override the title.
> 
> The first time I read this, it sounded like you could return something here but VoiceOver would let the description override - but actually this method needs to return nothing if there's alt text. To be more clear, maybe say "if there's alternative text that's meant to override the visible text, this will return the empty string so the alternative text can override the visible text."
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1830
> > +        AccessibilityText text = textOrder[k];
> 
> const AccessibilityText& text = textOrder[k]; - and similarly in every other method, so you don't do an unnecessary copy.
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1845
> > +- (NSString *)accessibilityDescription
> 
> How about a complementary comment here, indicating this is gathered from alternative text.
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1854
> > +        // If we encounter alternative text, then do not expose the title.
> 
> Comment is wrong
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1863
> > +        if (text.textSource == TitleTagText)
> 
> Just curious, why does "title" get promoted to description but not "summary"?
> 

Developers *frequently* confuse title as a means to "label" an element. Hence WAI-ARIA says as a last resort, use the title to describe the element. They don't say that for summary

> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:1891
> > +    return [NSString string];
> 
> I wonder if it might be more readable to combine these into one method that takes one pass over the string array and computes all three at once:
> 
> accessibilityTextMac(const Vector<AccessibilityText>& textOrder, String* title, String* description, String* help)
> {
>     *title = 0;
>     *description = 0;
>     *help = 0;
> 
>     unsigned length = textOrder.size();
>     for (unsigned k = 0; k < length; k++) {
>         const AccessibilityText& text = textOrder[k];
> 
>         if (text.textSource == VisibleText && !description)
>             title = &text.text;
>         else if (text.textSource == AlternativeText)
>             description = &text.text;
>         ...
>     }
> }

I thought about this, but as you noted it gets messy. I'm not sure which is the best way to go, but I feel separate methods is at least clean

> 
> It would only update pointers when it finds each one, so it's not actually not less efficient than what you have. And it'd keep all of the logic in one place rather than the logic of accessibilityDescription heavily depending on the logic of accessibilityTitle and so on.
> 
> Just an idea. If that looks uglier then I'll keep thinking about how else we could avoid duplicating the logic.
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1138
> > +        const AtomicString& ariaLabel = documentElement->getAttribute(aria_labelAttr);
> 
> Why not call ariaAccessibilityDescription here?

The thought that an HTML document could be labelled by an element within itself had not occurred to me. I suppose we could also to labeled-by in another patch

> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1231
> > +    String describedBy = ariaDescribedByAttribute();
> 
> I think aria-describedby should come ahead of aria-help.

I would disagree here. I guess it gets to the heart of what is "help" text. Aria-describedby is not necessarily just "help" text. The fact that it points to something else on screen means that it really is just a relationship to other information on the screen. Aria-help allows you to specifically craft a message designed just to "help" the user understand what to do, or what will happen in regards to this element.

That's the reason I gave aria-describedby the SummaryTextTag instead of HelpText tag, so a platform could decide what to do with both if need be.

[Dominic Mazzoni]

(In reply to comment #15)
> Developers *frequently* confuse title as a means to "label" an element. Hence WAI-ARIA says as a last resort, use the title to describe the element. They don't say that for summary

You're right. Agreed.

> > Why not call ariaAccessibilityDescription here?
> 
> The thought that an HTML document could be labelled by an element within itself had not occurred to me. I suppose we could also to labeled-by in another patch

I'm still in favor of this just for symmetry - I think the rules for a document should be as similar as possible to the rules for any other element. It's the least confusing for developers and users.

> > 
> > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1231
> > > +    String describedBy = ariaDescribedByAttribute();
> > 
> > I think aria-describedby should come ahead of aria-help.
> 
> I would disagree here. I guess it gets to the heart of what is "help" text. Aria-describedby is not necessarily just "help" text. The fact that it points to something else on screen means that it really is just a relationship to other information on the screen. Aria-help allows you to specifically craft a message designed just to "help" the user understand what to do, or what will happen in regards to this element.
> 
> That's the reason I gave aria-describedby the SummaryTextTag instead of HelpText tag, so a platform could decide what to do with both if need be.

Good point.  Since aria-help isn't actually specified in the spec yet, we can let platforms decide.

- Dominic

[chris fleizach]

(In reply to comment #16)
> (In reply to comment #15)
> > Developers *frequently* confuse title as a means to "label" an element. Hence WAI-ARIA says as a last resort, use the title to describe the element. They don't say that for summary
> 
> You're right. Agreed.
> 
> > > Why not call ariaAccessibilityDescription here?
> > 
> > The thought that an HTML document could be labelled by an element within itself had not occurred to me. I suppose we could also to labeled-by in another patch
> 
> I'm still in favor of this just for symmetry - I think the rules for a document should be as similar as possible to the rules for any other element. It's the least confusing for developers and users.

https://bugs.webkit.org/show_bug.cgi?id=98784

> 
> > > 
> > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1231
> > > > +    String describedBy = ariaDescribedByAttribute();
> > > 
> > > I think aria-describedby should come ahead of aria-help.
> > 
> > I would disagree here. I guess it gets to the heart of what is "help" text. Aria-describedby is not necessarily just "help" text. The fact that it points to something else on screen means that it really is just a relationship to other information on the screen. Aria-help allows you to specifically craft a message designed just to "help" the user understand what to do, or what will happen in regards to this element.
> > 
> > That's the reason I gave aria-describedby the SummaryTextTag instead of HelpText tag, so a platform could decide what to do with both if need be.
> 
> Good point.  Since aria-help isn't actually specified in the spec yet, we can let platforms decide.
> 
> - Dominic

[chris fleizach]

Created attachment 168977 [details]
patch

This patch passes all mac tests with a minor adjustment to one test
Doing this exercise definitely exposed some weirdness and inconsistency with how various tags and attributes are applied for different elements.
it will be good to fix those after this gets in

[WebKit Review Bot]

Attachment 168977 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1232:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14395204

[WebKit Review Bot]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14397227

[kov's GTK+ EWS bot]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14384264

[Early Warning System Bot]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14384267

[Early Warning System Bot]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14384274

[Peter Beverloo (cr-android ews)]

Comment on attachment 168977 [details]
patch

Attachment 168977 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14392234

[chris fleizach]

Created attachment 170316 [details]
patch

[chris fleizach]

this patch addresses the original problem now that we've refactored how text is determined.

[Dominic Mazzoni]

Comment on attachment 170316 [details]
patch

Unofficial review, looks great. Thanks for doing the refactoring work,
it makes this fix so easy and clean now. Just one small idea.

View in context: https://bugs.webkit.org/attachment.cgi?id=170316&action=review

> LayoutTests/platform/mac/accessibility/aria-label-overrides-visible-text.html:6
> +<a tabindex="0" id="link" href="#" aria-label="LINK">test1</a>

It would be even more clear if you had two links here to test - the first one just has text, the second one overrides it with aria-label.

[chris fleizach]

http://trac.webkit.org/changeset/132404

Also added extra test scenario as Dominic suggested