Bug 135116 – AX: HTML5 landmark (and related) elements should not be ignored.

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

[Takeshi Kurosawa]

Created attachment 235216 [details]
Testcase

WebKit ignores and doesn't expose following HTML5 landmark (and related) elements when they have block render object children:

- header (LandmarkBannerRole)
- aside (LandmarkComplementaryRole)
- footer (FooterRole)
- address (LandmarkContentInfoRole)
- main (LandmarkMainRole)
- nav (LandmarkNavigationRole)
- article (DocumentArticleRole)
- section (DocumentRegionRole)
- dt (DescriptionListTermRole)
- dd (DescriptionListDetailRole)

We should not ignore these elements and should expose them.
# Although dt and dd are not related to landmark, we should expose.

[Radar WebKit Bug Importer]

<rdar://problem/17746405>

[Takeshi Kurosawa]

Created attachment 235219 [details]
Patch

Patch.

[WebKit Commit Bot]

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


ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1257:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 8 files


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

[chris fleizach]

Comment on attachment 235219 [details]
Patch

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

patch looks ok besides style issue. thanks

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1257
> +        case AudioRole:

don't indent here

[Takeshi Kurosawa]

Created attachment 235248 [details]
Patch (indetation fix)

Fixed indentation.

[James Craig]

Comment on attachment 235248 [details]
Patch (indetation fix)

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:320
> +        || role == FooterRole

We definitely don't want this. <footer> can be used hundreds of times per page. For example, each post in a Facebook feed can have a footer. There's another open bug about Footer versus ContentInfo. This would make all of them landmarks. 

You only want footer to map to content info when it's not a descendant of an article or section: 
http://www.w3.org/html/wg/drafts/html/CR/dom.html#sec-strong-native-semantics

Just pull this out and it'd be okay to address the rest in a separate bug.

[James Craig]

related to bug 109013

[James Craig]

(In reply to comment #6)
> You only want footer to map to content info when it's not a…

Should have read:
> You only want footer to *be a landmark* when it's not a…

[Takeshi Kurosawa]

Created attachment 235286 [details]
Patch (drop footer element)

Drop footer element as per review comment

[Takeshi Kurosawa]

Created attachment 235287 [details]
Patch (drop footer element)

Oops! The previous patch contains wrong ChangeLogs.

[chris fleizach]

Comment on attachment 235287 [details]
Patch (drop footer element)

thanks

[James Craig]

Thanks Takeshi!

[James Craig]

Let us know if you need a CQ+, too.

[James Craig]

Comment on attachment 235287 [details]
Patch (drop footer element)

Adding CQ+

[WebKit Commit Bot]

Comment on attachment 235287 [details]
Patch (drop footer element)

Clearing flags on attachment: 235287

Committed r173428: <http://trac.webkit.org/changeset/173428>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Steve Faulkner]

(In reply to comment #6)
> (From update of attachment 235248 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235248&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:320
> > +        || role == FooterRole
> 
> We definitely don't want this. <footer> can be used hundreds of times per page. For example, each post in a Facebook feed can have a footer. There's another open bug about Footer versus ContentInfo. This would make all of them landmarks. 
> 
> You only want footer to map to content info when it's not a descendant of an article or section: 
> http://www.w3.org/html/wg/drafts/html/CR/dom.html#sec-strong-native-semantics
> 
> Just pull this out and it'd be okay to address the rest in a separate bug.

- header (LandmarkBannerRole) is same 
"header element that is not a descendant of an article or section element."
http://www.w3.org/html/wg/drafts/html/CR/dom.html#sec-strong-native-semantics

the original implementation of these requirements (for header/footer) were in webkit, Firefox and Chrome followed suit.

[James Craig]

Steve, if that is not working as expected, we'll need a second bug for that work.