Closed Bug 634004 Opened 13 years ago Closed 8 years ago

Make <details> and <summary> accessible

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: davidb, Assigned: MarcoZ)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 6 obsolete files)

Steve can you fill in the details?
summary and details elements [1]

focus and keyboard interaction

The summary element should be focusable by default.

The details element should not focusable by default.

Pressing the spacebar key when the summmary element has focus will show the details element content if the content is hidden. If the details element content is showing and the summary element has focus, pressing the spacebar key will hide the details element content.

Role, name, state and property mapping

The summary element should be mapped to a disclosure triangle role in accessibility APIs that have such a role. For example the Mac accessibility API includes the AXDisclosureTriangle role. In accessibility APIs that do not have such a fine grained role, the summary element should be mapped to a button role. The role mapping table contains recommended mappings for the summary element [2].

The accessible name for the summary element is the text content of the summary element.

When the details element content is hidden, the state of the content should be reflected by an accessible state or property. For example, in the Mac accessibility API on the summary element (AXDisclosureTriangle), set AXValue property to 0. When the details element content is showing on the summary element (AXDisclosureTriangle), set the AXValue property to 1. The hidden and showing states of the details element is reflected by the absence or presence of the open attribute (mapping details)[3].

[1] http://dev.w3.org/html5/html-api-map/overview.html#examples-sum
[2]http://dev.w3.org/html5/html-api-map/overview.html#sum
[3] http://dev.w3.org/html5/html-api-map/overview.html#att-open
(In reply to comment #1)
> summary and details elements [1]
> 
> focus and keyboard interaction
> 
> The summary element should be focusable by default.
> 
> The details element should not focusable by default.
> 
> Pressing the spacebar key when the summmary element has focus will show the
> details element content if the content is hidden. If the details element
> content is showing and the summary element has focus, pressing the spacebar key
> will hide the details element content.

It sounds it needs another bug for content module. Mounir, is it known issue?
(In reply to steve faulkner from comment #1)
> summary and details elements [1]
> 
> focus and keyboard interaction
> 
> The summary element should be focusable by default.
> 
> The details element should not focusable by default.
> 
> Pressing the spacebar key when the summmary element has focus will show the
> details element content if the content is hidden. If the details element
> content is showing and the summary element has focus, pressing the spacebar
> key will hide the details element content.
> 
> Role, name, state and property mapping
> 
> The summary element should be mapped to a disclosure triangle role in
> accessibility APIs that have such a role. For example the Mac accessibility
> API includes the AXDisclosureTriangle role. In accessibility APIs that do
> not have such a fine grained role, the summary element should be mapped to a
> button role. The role mapping table contains recommended mappings for the
> summary element [2].
> 
> The accessible name for the summary element is the text content of the
> summary element.
> 
> When the details element content is hidden, the state of the content should
> be reflected by an accessible state or property. For example, in the Mac
> accessibility API on the summary element (AXDisclosureTriangle), set AXValue
> property to 0. When the details element content is showing on the summary
> element (AXDisclosureTriangle), set the AXValue property to 1. The hidden
> and showing states of the details element is reflected by the absence or
> presence of the open attribute (mapping details)[3].
> 
> [1] http://dev.w3.org/html5/html-api-map/overview.html#examples-sum
> [2]http://dev.w3.org/html5/html-api-map/overview.html#sum
> [3] http://dev.w3.org/html5/html-api-map/overview.html#att-open

udpated acc API mapping spec links:

open attribute: http://w3c.github.io/aria/html-aam/html-aam.html#att-open-details
summary element: http://w3c.github.io/aria/html-aam/html-aam.html#el-summary
details element: http://w3c.github.io/aria/html-aam/html-aam.html#el-details
(In reply to steve faulkner from comment #5)

> open attribute:
> http://w3c.github.io/aria/html-aam/html-aam.html#att-open-details
> summary element: http://w3c.github.io/aria/html-aam/html-aam.html#el-summary

it is definitely clickable but I'm not sure if it's a button. It doesn't really look like a button (at least in Chrome) and ATs usually trim all buttons content, which may be a problem.

should we keep HTML:summary accessible at all? we could say instead that HTML:details is expanable/collapseble grouping role with expand/collapse action.
(In reply to alexander :surkov from comment #6)
> (In reply to steve faulkner from comment #5)
> 
> > open attribute:
> > http://w3c.github.io/aria/html-aam/html-aam.html#att-open-details
> > summary element: http://w3c.github.io/aria/html-aam/html-aam.html#el-summary
> 
> it is definitely clickable but I'm not sure if it's a button. It doesn't
> really look like a button (at least in Chrome) and ATs usually trim all
> buttons content, which may be a problem.

Reason why its a button (with exapnded=true|false) is that it is closest disclosure triangle (mac AX)

currently it is implemented as button in chrome
this is example using aria http://codepen.io/stevef/pen/jiCBE

> should we keep HTML:summary accessible at all? we could say instead that
> HTML:details is expanable/collapseble grouping role with expand/collapse
> action.

then the details elements becomes focusable? is this a known UI pattern?
ok, that sounds reasonable for Chrome implementation (not sure what Firefox one will be) except it doesn't address the issue that button's content has to be flat, since pushbutton in MSAA world doesn't have children. More closest match would be ROLE_SYSTEM_BUTTONMENU. On the other matter should we expose any relations between summary and details content? Do we need an accessible for HTML:details then?
(In reply to alexander :surkov from comment #8)
> ok, that sounds reasonable for Chrome implementation (not sure what Firefox
> one will be) except it doesn't address the issue that button's content has
> to be flat, since pushbutton in MSAA world doesn't have children. More
> closest match would be ROLE_SYSTEM_BUTTONMENU. On the other matter should we
> expose any relations between summary and details content? Do we need an
> accessible for HTML:details then?

Hi Alex, interested in the issue of MSAA world button not having children.
1. why are we still constraining design to MSAA world when MSAA is dying or being replaced crhome/firefox?
2. a practical example http://s.codepen.io/stevef/debug/yYrNvo
the acc tree exposed the heading in the button in Firefox and it is announced by NVDA.

on HTML:details , I think that it should be exposed as a group (because it groups content :-) and their should be a control relationship between summary and details, but to be Practically useful the details element needs to have an accname otherwise it will be ignored by AT (from limited testing)

Having said that currently on Mac/safari details is hiden from acc tree. We really need to get interop on this feature, whatever form that may take.
(In reply to steve faulkner from comment #9)

> Hi Alex, interested in the issue of MSAA world button not having children.
> 1. why are we still constraining design to MSAA world when MSAA is dying or
> being replaced crhome/firefox?

you're right, it's rather firefox/windows screen readers legacy. iirc there was a similar issue on mac.

> 2. a practical example http://s.codepen.io/stevef/debug/yYrNvo
> the acc tree exposed the heading in the button in Firefox and it is
> announced by NVDA.

good :) then NVDA is whitelisted they don't rely on that behavior :) Firefox makes a trick, if it sees the button's tree is not trivial then it just exposes everything. I guess we probably could proceed with a button, but wouldn't be ROLE_SYSTEM_BUTTONMENU a better description for it?

> on HTML:details , I think that it should be exposed as a group (because it
> groups content :-) and their should be a control relationship between
> summary and details, 
> but to be Practically useful the details element needs
> to have an accname otherwise it will be ignored by AT (from limited testing)

It'd be good to hear from AT developers on this, I'm curious whether AT need a mechanism to find a content when the details was expanded.

> 
> Having said that currently on Mac/safari details is hiden from acc tree. We
> really need to get interop on this feature, whatever form that may take.

In general we don't have to require the browsers to expose that has no semantics.
Some thoughts on this:
1. The WAI-ARIA polyfill for summary/details in Steve's example here is very much to my liking: http://codepen.io/stevef/pen/jiCBE

2. I am against treating the details element as the focusable one. To me, this is merely a wrapper around everything that acts like a slightly enhanced div. I am more for properly mapping the summary element directly. And that is also what is focusable. As for the grouping having a label...Not sure this makes a lot of sense. Not all ATs actually indicate the end of a group (like the end of a field set). I am thinking that knowing where the expanded content starts is more important than knowing where it ends (see below).

3. Mac a11y trumps the heading information from the button title. I currently only have the two ARIA-polyfilled examples Steve provided handy, so don't know what this would do without the ARIA polyfill. Steve, could you quickly hack together a version of http://s.codepen.io/stevef/debug/yYrNvo taking away the ARIA? I'd do it myself, but the Codepen editor isn't really accessible. :-(

4. As for the accessible relations: The controls/controlled_by relation sounds appropriate to mek, but it would need to be smart enough to do a 1 to n relation to all the other contents inside the details element. Alternatively, only the first non-summary element gets that relation between it and the summary set, as kind of the starting point. If screen readers provide a way to jump back and forth, they always have a defined starting point.

Thoughts?
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(faulkner.steve)
(In reply to Marco Zehe (:MarcoZ) from comment #13)
Marco here is un an arified version:
http://codepen.io/stevef/pen/zrMraG

but this is simply:

<details>
<summary><h1>test</test></summary>
<p>some content</p>
</details>

is this what you wanted?
Yes, thanks Steve!
Flags: needinfo?(faulkner.steve)
Comment on attachment 8689465 [details] [diff] [review]
Implement toggling open details by keyboard (WIP)

Hi! Is this patch still relevant, or did it make it into the main patch queue for bug 591737? I don't see a tab stop on the summary element, so I am suspecting no, this is still needed, correct?
Flags: needinfo?(tlin)
The user can switch to the main <summary> by tab key, and toggle the
<details> by either 'spacebar' key or 'enter' key.
Comment on attachment 8689465 [details] [diff] [review]
Implement toggling open details by keyboard (WIP)

The patch in this bug is still needed to make the <details> toggle by keyboard. 
I've rebased the patch against the latest master branch. 

I know nothing about a11y, so I'm no sure the patch meets all the requirements to make <details> and <summary> accessible. Any guidance or feedback is welcome. A hand from accessibility team to take the patch across the finish line is even better.
Attachment #8689465 - Attachment is obsolete: true
Flags: needinfo?(tlin)
The user can switch to the main <summary> by tab key, and toggle the
<details> by either 'spacebar' key or 'enter' key.
Attachment #8717327 - Attachment is obsolete: true
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) (Lunar New Year 2/6 - 2/14, slow response) from comment #18)
> The patch in this bug is still needed to make the <details> toggle by
> keyboard. 
> I've rebased the patch against the latest master branch. 

Thank you!

> I know nothing about a11y, so I'm no sure the patch meets all the
> requirements to make <details> and <summary> accessible. Any guidance or
> feedback is welcome. A hand from accessibility team to take the patch across
> the finish line is even better.

Yes, the API mappings for accessibility are still missing. Here are some pointers how I'd go about it:

1. in accessible/base/Role.h, add new roles DETAILS and SUMMARY to the enum and update the LAST_ROLE constant accordingly.

2. accessible/base/RoleMap.h: Add a mapping for these new roles, using GROUP and PUSHBUTTON as the base respectively, but for SUMMARY, change the NSAccessibility mapping to read NSAccessibilityGroupRole, too.

3. accessible/interfaces/nsIAccessibleRole.idl: Add the role to.

4. accessible/mac/mozAccessible.mm, add the mappings to the big case statement in the subrole method: @"AXdetails" for DETAILS and @"AXSummary" for SUMMARY.

5. accessible/html/HTMLElementAccessibles.cpp/.h: Add a new class for the summary element, overriding the following methods:

a) nativeState to return states including states::COLLAPSED or states::EXPANDED respectively, depending on the actual state whether details are currently hidden or shown.

b) ActionNameAt [0] to return either "expand" if it is collapsed, or "collapse" if it is expanded. Even though it is a button, the action name should be less generic than just "press" here I think.

6. Here's where it gets fuzzy for me, since I am more a tester and usability advisory person than ahardcore coder. I see two possibilities:

a) accessible/base/MarkupMap.h: Add new rules for details, which creates an HTMLGroupBoxAccessible with role DETAILS, and a summary, which creates an HTMLSummaryAccessible (see above) and has a role of SUMMARY.

or

b) in Layout, add the AccessibleType method returning a11y::eHTMLGroupBoxType for the details frame, and add a new a11y:eSummaryType for summary and add that. And then, in accessible/base/NSAccessibilityService.cpp, add a rule for the eSummaryType to add an HTMLSummaryElementAccessible created above.

Since our most accessibles are created from layout frames if they are present, I bet b) is the way surkov would want this to happen. But I am uncertain.

7. Add tests to accessible/tests/mochitest for the tree and the events when clicking the summary element so it sends state change events for collapse/expand.

And I probably forgot something... But this is the general direction.
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) (Chinese New Year 2/6 - 2/14, slow response) from comment #19)
> Created attachment 8717332 [details] [diff] [review]
> Implement toggling open details by keyboard (v3)
> 
> The user can switch to the main <summary> by tab key, and toggle the
> <details> by either 'spacebar' key or 'enter' key.

the approach looks good with me, can you please proceed with review from a layout peer?
Flags: needinfo?(surkov.alexander)
Would you like to give it a shot for a11y part outlined by Marco in comment #20?
Flags: needinfo?(tlin)
For now, I can proceed with adding some test cases with the patch attached comment #19, and land this part. 

I could give the a11y part a shot in comment #20, but I'd better do this after I implement all the spec stuff for details & summary and fix all known crashes.

FWIW, this comment need to be considered when doing the a11y work. https://bugzilla.mozilla.org/show_bug.cgi?id=591737#c133
Flags: needinfo?(tlin)
Comment on attachment 8717332 [details] [diff] [review]
Implement toggling open details by keyboard (v3)

Review of attachment 8717332 [details] [diff] [review]:
-----------------------------------------------------------------

asking Boris as he was a primary reviewer of bug 634004
Attachment #8717332 - Flags: review?(bzbarsky)
Comment on attachment 8717332 [details] [diff] [review]
Implement toggling open details by keyboard (v3)

This needs tests.

>+  const bool defaultFocusable = !aWithMouse;

This is not using the pref that <input> does, right?  Please document why not.

Should non-main-summary focusability be special too?  If so, please document why; I'm not sure it should.
Attachment #8717332 - Flags: review?(bzbarsky) → review-
No longer blocks: ship-details-summary
Attachment #8717332 - Attachment is obsolete: true
Per bug 1226455, we'll need this before shipping details and summary.  I experiment the direction in comment 20 a bit today, but still feel I might not be the best person to take this bug.  David, could your team help on this?

BTW, I turn on VoiceOver on Nightly without e10s and tab through links, and it doesn't read the content of the link. Is this a known issue?
Flags: needinfo?(dbolter)
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #26)
> Per bug 1226455, we'll need this before shipping details and summary.  I
> experiment the direction in comment 20 a bit today, but still feel I might
> not be the best person to take this bug.  David, could your team help on
> this?

Sorry for the delay. I'm still trying to figure out who might work on this.

> 
> BTW, I turn on VoiceOver on Nightly without e10s and tab through links, and
> it doesn't read the content of the link. Is this a known issue?

Maybe :)  It is best to have VO running before FF. In general we don't support Mac well yet for accessibility, just Windows and Linux (and Mobile).
I don't know anybody who can jump on this now.

TYLin are you able to test your work on a different platform?

Would it help if we wrote failing a11y mochitests?
Flags: needinfo?(dbolter) → needinfo?(tlin)
David, I mainly use Mac OS X for my development, and Linux rarely.  I guess failing a11y mochitests might help little since we still need to implement the all the things Marco mentioned in comment 20.

Marco, given that the Firefox 49 is shipping in 6/6, we cannot complete the a11y api support in time.  If we ship details and summary without a11y api support, would it become a regression for a11y users on those pages already using details and summary like [1]?  If these pages are decent, they ought to be using js to mimic the behavior on those browser without details and summary support.

If the a11y part is not a 100% blocker to ship details and summary, I really want to ship it to gain more feedback on real websites for the DOM and rendering part.

[1] https://developer.microsoft.com/en-us/microsoft-edge/platform/faq/
Flags: needinfo?(tlin)
Flags: needinfo?(mzehe)
Flags: needinfo?(dbolter)
It would be a real bummer if this wasn't accessible from the start. I'll take a stab at it and enlist help from surkov if I need it. Going with the simpler of the two approaches.
Assignee: nobody → mzehe
Status: NEW → ASSIGNED
Flags: needinfo?(mzehe)
Flags: needinfo?(dbolter)
Attached patch WIP - doesn't compile yet (obsolete) — Splinter Review
Alex, can you take a first look at this to confirm that the approach is generally correct? I know that it doesn't compile yet. I have not yet managed to untangle how I need to query what to actually get from mContent to the Details element through the summary element. See two instances of this in HTMLSummaryAccessible, which do not work.

But I would like to know if the general direction is how this should go. And if you have a hint on how I need to construct this to get to the methods, that would help, you know this better than I.
Attachment #8756796 - Flags: feedback?(surkov.alexander)
Comment on attachment 8756796 [details] [diff] [review]
WIP - doesn't compile yet

Review of attachment 8756796 [details] [diff] [review]:
-----------------------------------------------------------------

yep, it looks good overall

::: accessible/html/HTMLElementAccessibles.h
@@ +103,5 @@
> +
> +  HTMLSummaryAccessible(nsIContent* aContent, DocAccessible* aDoc);
> +
> +  // Accessible
> +  virtual mozilla::a11y::role NativeRole() override;

you don't have to provide NativeRole() implementation since it should go from MARKUPMAP stuff
Attachment #8756796 - Flags: feedback?(surkov.alexander) → feedback+
(In reply to alexander :surkov from comment #32)
> you don't have to provide NativeRole() implementation since it should go
> from MARKUPMAP stuff

Ah, right. :-)

BTW found the correct way to get to my elements. :-)
Attached patch WIP v2 (obsolete) — Splinter Review
Alex, this compiles, but it doesn't seem to fire the state change event when togging the details with space or enter. NVDA remains silent, and only after I ask do I get the changed state of the button reported.

Do you see what's wrong with this patch? Note that it doesn't contain tests yet because I want it to work with a screen reader first. But perhaps you see something in this approach which tells you why this doesn't work.
Attachment #8756796 - Attachment is obsolete: true
Flags: needinfo?(surkov.alexander)
Attachment #8756934 - Flags: feedback?(surkov.alexander)
Comment on attachment 8756934 [details] [diff] [review]
WIP v2

Review of attachment 8756934 [details] [diff] [review]:
-----------------------------------------------------------------

I think the issue is OpenStateChange is handled for XUL trees only in nsRootAccessible, but I wouldn't rely on DOM events. Instead I would add a new method like nsAccesibilityService::NotifyOfExpand(nsIContent* aTarget, bool aIsExpanded) and would call it from HTMLSummaryElement class directly.
Attachment #8756934 - Flags: feedback?(surkov.alexander) → feedback+
Flags: needinfo?(surkov.alexander)
Alex, I tried as you suggested, and still wasn't getting a spoken state change event. I then took a look at the button accessibles before and after a toggle. Here is the NVDA info for each:

Before toggle:
Developer info for navigator object:
name: u'Test'
role: ROLE_BUTTON
states: STATE_COLLAPSED, STATE_FOCUSABLE, STATE_FOCUSED
isFocusable: True
hasFocus: True
Python object: <NVDAObjects.Dynamic_ButtonMozillaIAccessible object at 0x050BD7D0>
Python class mro: (<class 'NVDAObjects.Dynamic_ButtonMozillaIAccessible'>, <class 'NVDAObjects.IAccessible.Button'>, <class 'NVDAObjects.IAccessible.mozilla.Mozilla'>, <class 'NVDAObjects.IAccessible.ia2Web.Ia2Web'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <type 'object'>)
description: u''
location: (16, 154, 2968, 140)
value: None
appModule: <'firefox' (appName u'firefox', process ID 2896) at address 52bae50>
appModule.productName: u'Nightly'
appModule.productVersion: u'49.0a1'
TextInfo: <class 'NVDAObjects.IAccessible.IA2TextTextInfo'>
windowHandle: 1771414L
windowClassName: u'MozillaWindowClass'
windowControlID: 0
windowStyle: 399441920
windowThreadID: 7612
windowText: u'Testing Summary and details - Nightly'
displayText: u''
IAccessibleObject: <POINTER(IAccessible2) ptr=0x11677304 at 5120080>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=1771414, objectID=-4, childID=-254889600
IAccessible accName: u'Test'
IAccessible accRole: ROLE_SYSTEM_PUSHBUTTON
IAccessible accState: STATE_SYSTEM_COLLAPSED, STATE_SYSTEM_FOCUSED, STATE_SYSTEM_FOCUSABLE, STATE_SYSTEM_VALID (1049604)
IAccessible accDescription: u''
IAccessible accValue: None
IAccessible2 windowHandle: 1771414
IAccessible2 uniqueID: -254889600
IAccessible2 role: ROLE_SYSTEM_PUSHBUTTON
IAccessible2 states: IA2_STATE_SELECTABLE_TEXT, IA2_STATE_OPAQUE (5120)
IAccessible2 attributes: u'margin-left:0px;text-align:start;text-indent:0px;formatting:block;margin-right:0px;tag:summary;margin-top:0px;margin-bottom:0px;display:list-item;line-number:1;'

After toggle:
Developer info for navigator object:
name: u'Test'
role: ROLE_BUTTON
states: STATE_EXPANDED, STATE_FOCUSABLE, STATE_FOCUSED
isFocusable: True
hasFocus: True
Python object: <NVDAObjects.Dynamic_ButtonMozillaIAccessible object at 0x0524DE30>
Python class mro: (<class 'NVDAObjects.Dynamic_ButtonMozillaIAccessible'>, <class 'NVDAObjects.IAccessible.Button'>, <class 'NVDAObjects.IAccessible.mozilla.Mozilla'>, <class 'NVDAObjects.IAccessible.ia2Web.Ia2Web'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <type 'object'>)
description: u''
location: (16, 154, 2968, 140)
value: None
appModule: <'firefox' (appName u'firefox', process ID 2896) at address 52bae50>
appModule.productName: u'Nightly'
appModule.productVersion: u'49.0a1'
TextInfo: <class 'NVDAObjects.IAccessible.IA2TextTextInfo'>
windowHandle: 1771414L
windowClassName: u'MozillaWindowClass'
windowControlID: 0
windowStyle: 399441920
windowThreadID: 7612
windowText: u'Testing Summary and details - Nightly'
displayText: u''
IAccessibleObject: <POINTER(IAccessible2) ptr=0x11679e0c at 51c38f0>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=1771414, objectID=-4, childID=-255865760
IAccessible accName: u'Test'
IAccessible accRole: ROLE_SYSTEM_PUSHBUTTON
IAccessible accState: STATE_SYSTEM_EXPANDED, STATE_SYSTEM_FOCUSED, STATE_SYSTEM_FOCUSABLE, STATE_SYSTEM_VALID (1049092)
IAccessible accDescription: u''
IAccessible accValue: None
IAccessible2 windowHandle: 1771414
IAccessible2 uniqueID: -255865760
IAccessible2 role: ROLE_SYSTEM_PUSHBUTTON
IAccessible2 states: IA2_STATE_SELECTABLE_TEXT, IA2_STATE_OPAQUE (5120)
IAccessible2 attributes: u'margin-left:0px;text-align:start;text-indent:0px;formatting:block;margin-right:0px;tag:summary;margin-top:0px;margin-bottom:0px;display:list-item;line-number:1;'

Take particular note of the IAccessibleObject and IAccessible2 UniqueID properties. They are *different*. Which means that toggling the Open state of the details causes us to recreate the accessible for the summary element. So no wonder I'm not getting any state change event, the accessible I am firing this for goes away.

So this seems to me like another instance of bug 1261484 or something related, some reframing that causes us to recreate the accessible.

What do we do?
Flags: needinfo?(surkov.alexander)
Chrome does not recreate the accessible for the summary element, it works as expected.
Attached patch WIP V3 (obsolete) — Splinter Review
This is the patch as it currently stands, but without tests still, so no review request yet. Waiting on reply from surkov on how to proceed. Ideally, bug 1261484 should be fixed so that we don't recreate those accessibles all the time. :-(
Attachment #8756934 - Attachment is obsolete: true
This implements the roles, states, and action names, but omits the state change event part that is currently made impossible by us recreating the html:summary accessible once it toggles the html:details open state. This is probably due to some reframing causing us to recreate the accessible. Suggest to move that to a separate bug and implement the basics now and the event later.

Review commit: https://reviewboard.mozilla.org/r/56870/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56870/
Attachment #8758615 - Flags: review?(surkov.alexander)
Attachment #8758167 - Attachment is obsolete: true
Flags: needinfo?(surkov.alexander)
Filed bug 1277201 for the STATE_CHANGE event issue.
Comment on attachment 8758615 [details]
MozReview Request: Bug 634004 - Implement accessibility API support for html:details and html:summary elements, r=surkov

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56870/diff/1-2/
That update changed the html:details mapping from a groupbox to a hypertext with the proper role. The other didn't work, and I also found that basing it on HTMLGroupboxAccessible wasn't useful since it was very specific fieldset/legend logic in there. Tests passed locally. New try push is at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42d6fca0d317
Comment on attachment 8758615 [details]
MozReview Request: Bug 634004 - Implement accessibility API support for html:details and html:summary elements, r=surkov

https://reviewboard.mozilla.org/r/56870/#review53608

::: accessible/base/RoleMap.h:1356
(Diff revision 2)
>       USE_ROLE_STRING,
>       IA2_ROLE_TEXT_FRAME,
>       eNameFromSubtreeIfReqRule)
>  
> +ROLE(DETAILS,
> +     "grouping",

"details"

::: accessible/base/RoleMap.h:1364
(Diff revision 2)
> +     ROLE_SYSTEM_GROUPING,
> +     ROLE_SYSTEM_GROUPING,
> +     eNoNameRule)
> +
> +ROLE(SUMMARY,
> +     "pushbutton",

"summary"

::: accessible/html/HTMLElementAccessibles.cpp:179
(Diff revision 2)
> +HTMLSummaryAccessible::NativeState()
> +{
> +  uint64_t state = HyperTextAccessibleWrap::NativeState();
> +
> +  dom::HTMLSummaryElement* summary = dom::HTMLSummaryElement::FromContent(mContent);
> +  if (!summary) {

this shoudln't be faillabe in real life, up to you to keep it or remove it

::: accessible/html/HTMLElementAccessibles.cpp:193
(Diff revision 2)
> +  if (details->Open()) {
> +    state |= states::EXPANDED;
> +  } else {
> +    state |= states::COLLAPSED;
> +  }
> +    

nit: whitespace

::: accessible/html/HTMLElementAccessibles.cpp:203
(Diff revision 2)
> +// HTMLSummaryAccessible: Widgets
> +
> +bool
> +HTMLSummaryAccessible::IsWidget() const
> +{
> +  return true;

while I don't see anything particular bad in this, but what is a reason to make HTML summary a widget?
Attachment #8758615 - Flags: review?(surkov.alexander) → review+
Comment on attachment 8758615 [details]
MozReview Request: Bug 634004 - Implement accessibility API support for html:details and html:summary elements, r=surkov

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56870/diff/2-3/
Attachment #8758615 - Attachment description: MozReview Request: Bug 634004 - Implement accessibility API support for html:details and html:summary elements, r?surkov → MozReview Request: Bug 634004 - Implement accessibility API support for html:details and html:summary elements, r=surkov
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f77dcc75a129
Implement accessibility API support for html:details and html:summary elements, r=surkov
https://hg.mozilla.org/mozilla-central/rev/f77dcc75a129
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Marco, thank you for fixing this bug. \o/
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: