Closed Bug 1316285 Opened 3 years ago Closed 2 years ago

modify role mapping of header/footer to reflect those defined in the HTML Acc API spec.

Categories

(Core :: Disability Access APIs, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: faulkner.steve, Assigned: MarcoZ)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

After some feedback from SR users about the cognitive noise created by multiple <headers> and <footers> on a page, which are not descendants of either <article>  or <section>, the implementation requirement has been modified:

for header
https://w3c.github.io/html-aam/#el-header

for footer
https://w3c.github.io/html-aam/#el-footer

related spec bug
https://github.com/w3c/aria/issues/362


What these changes do is further restrict the mapping of header=banner and footer=contentinfo to reduce multiple instances.
This is an API thing, so moving to Core. I assume that you meant to CC Marco & Surkov, rather than implying that this is a "mentored" bug new contributors can easily start work on with the help of Marco & Surkov (which is what the mentor field is for), as there isn't really enough info in this bug for someone with little knowledge of our codebase to fix it.
Mentor: mzehe, surkov.alexander
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
CC'ing Alex to see if he has any ideas.
Flags: needinfo?(surkov.alexander)
So I think I good to restrict HTML footer/header mapping usage as HTML-AAM states. On implementation note:

the HTML spec [1] says:

"When the nearest ancestor sectioning content or sectioning root element is the body element, then it applies to the whole page, unless they are descendants of the main element."

Sectioning content is defined as:
"Sectioning content is content that defines the scope of headings and footers.
    article aside nav section "

Sectioning root is defined as:
"Certain elements are said to be sectioning roots, including blockquote and td elements. These elements can have their own outlines, but the sections and headings inside these elements do not contribute to the outlines of their ancestors.
    blockquote body details dialog fieldset figure td "

Then: introduce HTMLHeader/FooterAccessible class which would check all these stuff before we return accessible role or attributes. Also see [2] and [3] for code entry points.

[1] http://w3c.github.io/html/sections.html#the-header-element
[2] https://dxr.mozilla.org/mozilla-central/source/accessible/base/MarkupMap.h?q=MarkupMap.h&redirect_type=direct#66
[3] https://dxr.mozilla.org/mozilla-central/source/accessible/generic/HyperTextAccessible.cpp#1149
Blocks: html5a11y
Flags: needinfo?(surkov.alexander)
Whiteboard: [mentor=:surkov]
Assignee: nobody → mzehe
Status: NEW → ASSIGNED
Flags: needinfo?(mzehe)
Whiteboard: [mentor=:surkov]
Attachment #8901789 - Flags: review?(surkov.alexander) → review?(eitan)
Comment on attachment 8901789 [details]
Bug 1316285 - Expose role and landmark role for HTML header and footer only if they are direct descendants of the body tag, section otherwise.

https://reviewboard.mozilla.org/r/173222/#review178770

HTMLFooterAccessible and HTMLHeaderAccessible classes are identical except for the role they expose when not nested. I would combine them as one class and pass an argument in the constructor as to what role should be exposed.

Maybe HTMLTopLevelLandmarkAccessible? Or HTMLFooterHeaderAccessible?

::: accessible/html/HTMLElementAccessibles.cpp:216
(Diff revision 2)
> +NS_IMPL_ISUPPORTS_INHERITED0(HTMLFooterAccessible, HyperTextAccessible)
> +
> +role
> +HTMLFooterAccessible::NativeRole()
> +{
> +  // Only map header and footer if they are descendants of the body tag.

They will always be descendants of the body tag. How about: Header and footer tags don't retain their roles if they are nested in other landmarks.
Attachment #8901789 - Flags: review?(eitan)
Comment on attachment 8901789 [details]
Bug 1316285 - Expose role and landmark role for HTML header and footer only if they are direct descendants of the body tag, section otherwise.

https://reviewboard.mozilla.org/r/173222/#review179214

Looks great! Just some nits.

::: accessible/html/HTMLElementAccessibles.cpp:234
(Diff revision 4)
> +
> +  // No sectioning or sectioning root elements found.
> +  if (!parent) {
> +    if (mContent->IsHTMLElement(nsGkAtoms::header)) {
> +      return roles::HEADER;
> +    }

newline after return block.

::: accessible/html/HTMLElementAccessibles.cpp:249
(Diff revision 4)
> +HTMLHeaderOrFooterAccessible::LandmarkRole() const
> +{
> +  if (!HasOwnContent())
> +    return nullptr;
> +
> +  a11y::role r = const_cast<HTMLHeaderOrFooterAccessible*>(this)->Role();

Is this casting neccessary?

::: accessible/html/HTMLElementAccessibles.cpp:252
(Diff revision 4)
> +    return nullptr;
> +
> +  a11y::role r = const_cast<HTMLHeaderOrFooterAccessible*>(this)->Role();
> +  if (r == roles::HEADER) {
> +    return nsGkAtoms::banner;
> +  }

newline here too.
Attachment #8901789 - Flags: review?(eitan) → review+
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03245d48dbed
Expose role and landmark role for HTML header and footer only if they are direct descendants of the body tag, section otherwise. r=eeejay
https://hg.mozilla.org/mozilla-central/rev/03245d48dbed
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Duplicate of this bug: 1252799
You need to log in before you can comment on or make changes to this bug.