Bug 135116 - AX: HTML5 landmark (and related) elements should not be ignored.
Summary: AX: HTML5 landmark (and related) elements should not be ignored.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 109013
  Show dependency treegraph
 
Reported: 2014-07-21 06:22 PDT by Takeshi Kurosawa
Modified: 2014-09-09 08:29 PDT (History)
11 users (show)

See Also:


Attachments
Testcase (494 bytes, text/html)
2014-07-21 06:22 PDT, Takeshi Kurosawa
no flags Details
Patch (9.38 KB, patch)
2014-07-21 07:25 PDT, Takeshi Kurosawa
no flags Details | Formatted Diff | Diff
Patch (indetation fix) (9.34 KB, patch)
2014-07-21 14:29 PDT, Takeshi Kurosawa
jcraig: review-
Details | Formatted Diff | Diff
Patch (drop footer element) (8.66 KB, patch)
2014-07-22 05:49 PDT, Takeshi Kurosawa
no flags Details | Formatted Diff | Diff
Patch (drop footer element) (8.54 KB, patch)
2014-07-22 06:11 PDT, Takeshi Kurosawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takeshi Kurosawa 2014-07-21 06:22:24 PDT
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.
Comment 1 Radar WebKit Bug Importer 2014-07-21 06:22:56 PDT
<rdar://problem/17746405>
Comment 2 Takeshi Kurosawa 2014-07-21 07:25:29 PDT
Created attachment 235219 [details]
Patch

Patch.
Comment 3 WebKit Commit Bot 2014-07-21 07:27:46 PDT
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.
Comment 4 chris fleizach 2014-07-21 09:13:06 PDT
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
Comment 5 Takeshi Kurosawa 2014-07-21 14:29:45 PDT
Created attachment 235248 [details]
Patch (indetation fix)

Fixed indentation.
Comment 6 James Craig 2014-07-21 14:49:57 PDT
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.
Comment 7 James Craig 2014-07-21 14:51:20 PDT
related to bug 109013
Comment 8 James Craig 2014-07-21 23:55:03 PDT
(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…
Comment 9 Takeshi Kurosawa 2014-07-22 05:49:16 PDT
Created attachment 235286 [details]
Patch (drop footer element)

Drop footer element as per review comment
Comment 10 Takeshi Kurosawa 2014-07-22 06:11:48 PDT
Created attachment 235287 [details]
Patch (drop footer element)

Oops! The previous patch contains wrong ChangeLogs.
Comment 11 chris fleizach 2014-09-05 09:40:09 PDT
Comment on attachment 235287 [details]
Patch (drop footer element)

thanks
Comment 12 James Craig 2014-09-09 07:29:16 PDT
Thanks Takeshi!
Comment 13 James Craig 2014-09-09 07:30:01 PDT
Let us know if you need a CQ+, too.
Comment 14 James Craig 2014-09-09 07:34:32 PDT
Comment on attachment 235287 [details]
Patch (drop footer element)

Adding CQ+
Comment 15 WebKit Commit Bot 2014-09-09 08:10:08 PDT
Comment on attachment 235287 [details]
Patch (drop footer element)

Clearing flags on attachment: 235287

Committed r173428: <http://trac.webkit.org/changeset/173428>
Comment 16 WebKit Commit Bot 2014-09-09 08:10:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Steve Faulkner 2014-09-09 08:14:28 PDT
(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.
Comment 18 James Craig 2014-09-09 08:29:21 PDT
Steve, if that is not working as expected, we'll need a second bug for that work.