Last Comment Bug 666504 - Ignore role presentation on focusable elements
: Ignore role presentation on focusable elements
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla7
Assigned To: alexander :surkov
:
Mentors:
http://nvda.wikispaces.org/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-22 22:53 PDT by Michael Curran
Modified: 2011-06-30 23:46 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test html file with a focusable table with an aria role of presentation (362 bytes, text/plain)
2011-06-23 23:05 PDT, Michael Curran
no flags Details
patch (4.07 KB, patch)
2011-06-23 23:23 PDT, alexander :surkov
dbolter: review+
Details | Diff | Splinter Review
patch2 (8.34 KB, patch)
2011-06-28 22:10 PDT, alexander :surkov
mzehe: review+
Details | Diff | Splinter Review

Description Michael Curran 2011-06-22 22:53:22 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0) Gecko/20100101 Firefox/5.0
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0) Gecko/20100101 Firefox/5.0

Sometimes IAccessible2::role() can give back a role of 0 on tables and table cells. I did not see this occure in Firefox 4, but do see it in Firefox 5 and 6. Not sure about 7.
An impact of this is that NVDA no longer renders this content in its virtualBuffer at all, and object navigation becomes problematic.
The only place I have seen this bug occure so far is when trying to edit the page at:
http://nvda.wikispaces.org/

You will need to create a free wikispaces account first before you can edit the page.
All the edit controls are inside a table that exibits this bug.


Reproducible: Always

Steps to Reproduce:
1. In Firefox go to http://nvda.wikispaces.org/ and sign in if necessary (a free wikispaces account is needed). 
2. Click the "edit" link near the top of the page to start editing the NVDA - Home wiki page.
3. Tab to one of the editing controls such as the cancel button.
4. With an a11y inspection tool, or NVDA object navigation, note that the IAccessible2 role of the control's parent IAccessible is 0 (unknown). Also note if using NVDA that none of the editing controls appear in the virtualBuffer.

Actual Results:  
IAccessible2::role() gives back 0 and NVDA cannot render this content.

Expected Results:  
IAccessible2::role() should give back ROLE_SYSTEM_CELL or ROLE_SYSTEM_TABLE (depending on the actual object). NVDA should also render all the controls in its virtualBuffer.
Comment 1 Michael Curran 2011-06-23 00:33:33 PDT
Correction, the bug is in firefox 4, sorry about that.
Also,  I have tracked it down to:
If a table is focusable (say with tabindex="0" for example) [the table on nvda.wikispaces.org is focusable, not sure how though] and the table has  role="presentation", the table and its cells are still represented in the accessibility tree, but they all have an IAccessible2 role of 0.
I would like to understand why  an aria role of presentation forces  role to 0.  If it is that  role="presentation" nodes do not get filtered out of the accessibility tree if they are focusable, then they also need to maintain their original role.  Otherwise ATs such as NVDA will refuse to render them properly, or in this case not at all.
Comment 2 alexander :surkov 2011-06-23 02:20:37 PDT
Mick, what is a bug
1) presentation role should be ignored on presentation element or
2) the table at wiki is focusable while it shouldn't be?
Comment 3 Michael Curran 2011-06-23 17:59:23 PDT
Although I do not really understand the logic behind  forcing  the role to 0 on nodes with aria role presentation, I have worked around this in NVDA.
This bug can be closed as invalid if  others do not feel that this is problematic.
However, I will point out the following:
* Why mutate the role to 0 yet keep things like accessibleTable (row and column counts etc) available?
* Why does setting aria role of presentation on a "table" node  also cause the cells of the table ("TD" nodes) to also get a role of 0? This is one reason why  NVDA now has to descend in to all unknowns (role of 0) rather than perhaps just unknowns with an object attribute of role:presentation -- because the table cells in this example do not have role:presentation but do have a role of unknown (0).
* If  Gecko chooses not to filter out nodes with an aria role of presentation because they are focusable, I would assume this is because the node then obviously must have some importance. However,  the node is not very useful because its role is now lost.
Comment 4 alexander :surkov 2011-06-23 21:39:32 PDT
Mick, as I said on irc I think it's reasonable to keep role from markup when role presentation is used on focusable element, in other words, ignore role presentation on focusable nodes. Actually I agree that'll be a fix for the bug.

But I still wonder if focuable table in your example is expected and correct thing or that's a bug too.
Comment 5 alexander :surkov 2011-06-23 21:42:11 PDT
(In reply to comment #3)

> * Why does setting aria role of presentation on a "table" node  also cause
> the cells of the table ("TD" nodes) to also get a role of 0?

because cells make sense in context of the table, otherwise they are just text container and this is considered as wrong markup.
Comment 6 Michael Curran 2011-06-23 23:05:27 PDT
Created attachment 541607 [details]
test html file with a focusable table with an aria role of presentation
Comment 7 Michael Curran 2011-06-23 23:08:16 PDT
I have attached a testcase (html file containing a focusable table with an aria role of presentation). It has been made focusable with tabIndex="-1". Notice the table, header and cells are all role_unknown.
Re whether this is really an authoring bug: I'd probably think it was. I shal report this to wikispaces and see if they can take the tabIndex off the table.
Comment 8 alexander :surkov 2011-06-23 23:23:22 PDT
Created attachment 541611 [details] [diff] [review]
patch
Comment 9 alexander :surkov 2011-06-23 23:23:57 PDT
(In reply to comment #7)

> Re whether this is really an authoring bug: I'd probably think it was. I
> shal report this to wikispaces and see if they can take the tabIndex off the
> table.

Ok, thank you.
Comment 10 alexander :surkov 2011-06-23 23:24:44 PDT
changing summary to better reflect this bug is about.
Comment 11 Marco Zehe (:MarcoZ) 2011-06-23 23:41:34 PDT
Comment on attachment 541611 [details] [diff] [review]
patch

I wasn't asked for review, but does it make a difference to also test an usually non-focusable item made focusable by tabindex="0" additionally to one that *is* focusable by its very nature (like the button)? I would just like to see this covered.
Comment 12 alexander :surkov 2011-06-24 00:06:08 PDT
(In reply to comment #11)
> Comment on attachment 541611 [details] [diff] [review] [review]
> patch
> 
> I wasn't asked for review, 

Marco, even if you weren't asked you always can do review or do comments as module peer and assignee must address your concerns. 

The patch is trivial so probably two reviewers is too much. But thank you for looking into!

> but does it make a difference to also test an
> usually non-focusable item made focusable by tabindex="0" additionally to
> one that *is* focusable by its very nature (like the button)? I would just
> like to see this covered.

if you like. Should it be a test of focusable table tree hierarchy?
Comment 13 David Bolter [:davidb] ***PTO until 29th*** 2011-06-24 04:45:22 PDT
Comment on attachment 541611 [details] [diff] [review]
patch

r=me since this is an edge case. I don't think there is a perfect solution for all use cases.
Comment 14 Marco Zehe (:MarcoZ) 2011-06-27 07:14:32 PDT
> if you like. Should it be a test of focusable table tree hierarchy?

Yes, you can base it on Mick's testcase or use something simpler that gives us good test coverage here. I'd just like to be safe on this.
Comment 15 alexander :surkov 2011-06-28 22:10:28 PDT
Created attachment 542722 [details] [diff] [review]
patch2

new mochitests
Comment 16 alexander :surkov 2011-06-28 22:12:35 PDT
(In reply to comment #13)

> r=me since this is an edge case. I don't think there is a perfect solution
> for all use cases.

I didn't understand about edge case.
Comment 17 Marco Zehe (:MarcoZ) 2011-06-28 23:53:26 PDT
Comment on attachment 542722 [details] [diff] [review]
patch2

Thanks, that's what I had in mind! r=me
Comment 18 Trevor Saunders (:tbsaunde) 2011-06-29 17:26:44 PDT
pushed to try with 666504 606085  666337  429642   667396 as http://tbpl.mozilla.org/?tree=Try&rev=dbf9bbf7adc9
Comment 19 Trevor Saunders (:tbsaunde) 2011-06-30 10:28:55 PDT
landed http://hg.mozilla.org/mozilla-central/rev/7aa58a99a2e5

Note You need to log in before you can comment on or make changes to this bug.