The default bug view has changed. See this FAQ.

expose layout-guess: true object attribute on CSS table accessible

RESOLVED FIXED in mozilla11

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: steve faulkner, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug)

7 Branch
mozilla11
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (Windows NT 6.0; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928134238

Steps to reproduce:

viewed a test page that uses CSS display:table http://www.456bereastreet.com/lab/display-table/ further info: Using display:table has semantic effects in some screen readers http://www.456bereastreet.com/archive/201110/using_displaytable_has_semantic_effects_in_some_screen_readers/


Actual results:

div elements are represented in the accessibility tree as table elements. the following contains a screen shot of the accessibility tree in the DOM inspector for http://www.456bereastreet.com/lab/display-table/


Expected results:

the div elements should not be represented as table elements

Comment 1

6 years ago
OK, so display:table style is ALWAYS for layout purposes? If so, we can easily fix that by ALWAYS treating structures that have display:table and associated as if they were layout tables, by setting the appropriate object attribute to tell NVDA and Orca to ignore the table semantics.
(Reporter)

Comment 2

6 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #1)
> OK, so display:table style is ALWAYS for layout purposes? If so, we can
> easily fix that by ALWAYS treating structures that have display:table and
> associated as if they were layout tables, by setting the appropriate object
> attribute to tell NVDA and Orca to ignore the table semantics.

Hi Marco
you wrote;
 "OK, so display:table style is ALWAYS for layout purposes?"

I would suggest that the majority of uses are/will be to achieve a layout effect and we definitely should not encourage otherwise. It is also beneficial to make the change as it will bring firefox in line with how other browsers treat display:table.

Comment 3

6 years ago
FTR: I suspect VoiceOver in Safari, and NVDA in IE, because they do DOM parsing mostly rather than accessible tree parsing, will just not pick up display:table; style information and treat these like divs, that's why they are not being picked up as tables in those combinations mentioned in the article. I definitely agree that we should put the object attribute in, and encourage to NOT use these CSS styles for real data tables.
(Reporter)

Comment 4

6 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #3)
> FTR: I suspect VoiceOver in Safari, and NVDA in IE, because they do DOM
> parsing mostly rather than accessible tree parsing, will just not pick up
> display:table; style information and treat these like divs, that's why they
> are not being picked up as tables in those combinations mentioned in the
> article. I definitely agree that we should put the object attribute in, and
> encourage to NOT use these CSS styles for real data tables.

My understanding (from comments made by 	
Maciej Stachowiak of webkit) in regards to VoiceOver was that it didn't do any DOM parsing, taking all info from the accessibility API. Will follow up on that with the aplle folk.
(Reporter)

Comment 5

6 years ago
(In reply to steve faulkner from comment #4)
> (In reply to Marco Zehe (:MarcoZ) from comment #3)
> > FTR: I suspect VoiceOver in Safari, and NVDA in IE, because they do DOM
> > parsing mostly rather than accessible tree parsing, will just not pick up
> > display:table; style information and treat these like divs, that's why they
> > are not being picked up as tables in those combinations mentioned in the
> > article. I definitely agree that we should put the object attribute in, and
> > encourage to NOT use these CSS styles for real data tables.
> 
> My understanding (from comments made by 	
> Maciej Stachowiak of webkit) in regards to VoiceOver was that it didn't do
> any DOM parsing, taking all info from the accessibility API. Will follow up
> on that with the aplle folk.

FYI
James Craig confirmed that VoiceOver uses the Mac Accessibility API, not the DOM

Comment 6

6 years ago
As far as I know (and confirmed with a quick test), the layout-guess:true object attribute is already set for CSS display: table. In short, this allows a client to see the structure of the table via the a11y hierarchy if desired, while still notifying the client that it is a layout table.

By default, NVDA certainly honours this attribute in browse mode and when reporting focus ancestry.

Comment 7

6 years ago
I agree with Jamie, the current approach is we allow AT to decide. So either we should take decision on our side and never expose tables having layout-guess:true object attribute or do not change anything.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX

Updated

6 years ago
Blocks: 513662

Comment 8

6 years ago
(In reply to James Teh [:Jamie] from comment #6)
> By default, NVDA certainly honours this attribute in browse mode and when
> reporting focus ancestry.

Jamie, if you look at the very first table on the sample page Steve links to, you will see that NVDA, at least 2011.2 release, does indeed expose the full table semantics. Only the second table is being treated as a layout table and table semantics are suppressed. Both tables are built with CSS display:table; styles. Am I missing anything?

Comment 9

6 years ago
Damn. I was incorrect about display: table always exposing layout-guess: true. My test case was very simple and obviously triggered something else that caused Gecko to set layout-guess: true. I agree that layout-guess: true should *always* be exposed for CSS tables.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WONTFIX → ---

Updated

6 years ago
Blocks: 491681
No longer blocks: 513662

Comment 10

6 years ago
Ok, all we need to make nsHTMLTableAccessible::IsProbablyForLayout to check the tag name. If it's different from HTML table then expose layout-guess:true.
Status: REOPENED → NEW
Summary: css table accessibility representation → expose layout-guess: true object attribute on CSS table accessible
Whiteboard: [good first bug]
(Reporter)

Comment 11

6 years ago
ok, so the question that remains in my mind is why does CSS change the roles of the elements in the first place? This seems wrong to me.

Comment 12

6 years ago
(In reply to steve faulkner from comment #11)
> ok, so the question that remains in my mind is why does CSS change the roles
> of the elements in the first place? This seems wrong to me.

CSS affects on visual presentation, the purpose of accessibility is to expose to AT what user sees on the screen. For sighted users it doesn't matter what way was used to create a tabular representation therefore we expose those the same way for blind users.

Comment 13

6 years ago
(In reply to steve faulkner from comment #11)
> ok, so the question that remains in my mind is why does CSS change the roles
> of the elements in the first place? This seems wrong to me.
I guess you could equally ask why CSS display: none affects accessibility or why CSS formatting is exposed to accessibility. As much as this is all presentation info, it is still potentially relevant to ATs. In this particular case, we don't care or use that info, but that doesn't mean someone mightn't want to in future.
(Reporter)

Comment 14

6 years ago
(In reply to James Teh [:Jamie] from comment #13)
> (In reply to steve faulkner from comment #11)
> > ok, so the question that remains in my mind is why does CSS change the roles
> > of the elements in the first place? This seems wrong to me.
> I guess you could equally ask why CSS display: none affects accessibility or
> why CSS formatting is exposed to accessibility. As much as this is all
> presentation info, it is still potentially relevant to ATs. In this
> particular case, we don't care or use that info, but that doesn't mean
> someone mightn't want to in future.

hi James, I am not questioning whether the information should be conveyed, only the manner in which it is conveyed (by changing the roles of the elements). Roger Johannson talks about this more in http://www.456bereastreet.com/archive/201110/using_displaytable_has_semantic_effects_in_some_screen_readers/

Comment 15

6 years ago
As far as I can see Roger Johannson doesn't answer why display information should or shouldn't affect on the accessible role and hierarchy. All he said he didn't expect that it affects on accessible tree.

In Gecko we keep accessible tree closer to layout rather than DOM tree to make sure AT users gets same piece of information as sighted users do. ARIA is sort of exception of this rule, we apply it on top of accessible tree. But it's not clear with me that we should ignore some CSS to follow DOM closer.

Comment 16

6 years ago
(In reply to steve faulkner from comment #14)
> I am not questioning whether the information should be conveyed,
> only the manner in which it is conveyed (by changing the roles of the
> elements).
You can't convey table information without using roles and table interfaces. However, what we're proposing here is that Gecko expose it as a *layout* table. That's what the layout-guess:true attribute is for.

> Roger Johannson talks about this more in...
He talks about not wanting screen readers to treat them as tables when reporting them to the user. If layout-guess:true is exposed (as is being proposed in this bug), well-behaved screen readers *won't* treat them as tables when reporting them to the user.
(Reporter)

Comment 17

6 years ago
(In reply to James Teh [:Jamie] from comment #16)
> (In reply to steve faulkner from comment #14)
> > I am not questioning whether the information should be conveyed,
> > only the manner in which it is conveyed (by changing the roles of the
> > elements).
> You can't convey table information without using roles and table interfaces.
> However, what we're proposing here is that Gecko expose it as a *layout*
> table. That's what the layout-guess:true attribute is for.
> 
> > Roger Johannson talks about this more in...
> He talks about not wanting screen readers to treat them as tables when
> reporting them to the user. If layout-guess:true is exposed (as is being
> proposed in this bug), well-behaved screen readers *won't* treat them as
> tables when reporting them to the user.

hi James, alexander, 

"what we're proposing here is that Gecko expose it as a *layout* table. That's what the layout-guess:true attribute is for."

Sounds emminently reasonable to me. I am not arguing against this, only seeking to understand the reasoning, thanks for the replies!

Comment 18

6 years ago
All of this said, the name layout-guess:true is misleading in this case and afaik layout-guess is also non-standard. In future, we should perhaps consider a better named attribute (e.g. layout:{true,false,true-guess,false-guess}) and get it into the accessibility API standards.
(Reporter)

Comment 19

6 years ago
I am going to attempt document the IsProbablyForLayout algorithm any pointers to info?
(Reporter)

Comment 20

6 years ago
I should have said where I am going to document it: http://dev.w3.org/html5/html-api-map/#ta

Comment 21

6 years ago
(In reply to steve faulkner from comment #19)
> I am going to attempt document the IsProbablyForLayout algorithm any
> pointers to info?

I afraid we don't have any documentation on this. All we have is source code. I think I should find some time to document it.

Comment 22

6 years ago
(In reply to steve faulkner from comment #19)
> I am going to attempt document the IsProbablyForLayout algorithm any
> pointers to info?

Steve, I put algorithm at my blog http://asurkov.blogspot.com/2011/10/data-vs-layout-table.html
(Reporter)

Comment 23

6 years ago
(In reply to alexander surkov from comment #22)
> (In reply to steve faulkner from comment #19)
> > I am going to attempt document the IsProbablyForLayout algorithm any
> > pointers to info?
> 
> Steve, I put algorithm at my blog
> http://asurkov.blogspot.com/2011/10/data-vs-layout-table.html

Hi alex, many thanks! can i use this as the baisis for the documentation here: http://dev.w3.org/html5/html-api-map/#ta

Comment 24

6 years ago
(In reply to steve faulkner from comment #23)
> (In reply to alexander surkov from comment #22)
> > (In reply to steve faulkner from comment #19)
> > > I am going to attempt document the IsProbablyForLayout algorithm any
> > > pointers to info?
> > 
> > Steve, I put algorithm at my blog
> > http://asurkov.blogspot.com/2011/10/data-vs-layout-table.html
> 
> Hi alex, many thanks! can i use this as the baisis for the documentation
> here: http://dev.w3.org/html5/html-api-map/#ta

sure, that was a primary reason to blog about it to not make you to dig through our code

Comment 25

6 years ago
Hi everybody !!
let me to interfere with you in this discussion!!! 
to summing up your talks : we should check for CSS table to conclude that's for layout, but only for other HTML elements and none for HTML table, because (I think) in case of HTML table we should let the code to do its checks, unless we have strong reason for layout , right?

Comment 26

6 years ago
(In reply to Ouss. BADR from comment #25)
> Hi everybody !!
> let me to interfere with you in this discussion!!! 
> to summing up your talks : we should check for CSS table to conclude that's
> for layout, but only for other HTML elements and none for HTML table,
> because (I think) in case of HTML table we should let the code to do its
> checks, unless we have strong reason for layout , right?

Yes.

Updated

6 years ago
Assignee: nobody → oussamabadr

Comment 27

6 years ago
after some analysing,  I think we should create a new file like "nsHTMLCSSAccessible.cpp", coz we have to work on the CSS side for "layout-guess", 
What do you suggest? 

another think, is the platform just for windows vista and x86 or is it for  all platform? coz I didn't expect the platform specified for this bug!!!!! O_o
(Assignee)

Comment 28

6 years ago
(In reply to Ouss. BADR from comment #27)
> after some analysing,  I think we should create a new file like
> "nsHTMLCSSAccessible.cpp", coz we have to work on the CSS side for
> "layout-guess", 
> What do you suggest? 

that's not neccessary just check mContent->Tag() == nsGkAtoms::tag (patch comming in a few minutes its trivial)> 
>
 another think, is the platform just for windows vista and x86 or is it for 
> all platform? coz I didn't expect the platform specified for this bug!!!!!
> O_o

no, just artifat from the person filing using windows and so that being the default
(Assignee)

Comment 29

6 years ago
Created attachment 572407 [details] [diff] [review]
patch
Attachment #572407 - Flags: review?(surkov.alexander)

Comment 30

6 years ago
Hi Trevor!!

>  if (mContent->Tag() != nsGkAtoms::table)
>    RETURN_LAYOUT_ANSWER(true, "html table without table tag");

I think you should add a comment after this line like :
// Check for CSS table

...or something else which can explain what your lines are about ..

thks!!



thks !!!

Comment 31

6 years ago
.....sorry I mean before this line !

Comment 32

6 years ago
Comment on attachment 572407 [details] [diff] [review]
patch

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

::: accessible/src/html/nsHTMLTableAccessible.cpp
@@ +1387,5 @@
>      RETURN_LAYOUT_ANSWER(false, "Has role attribute, weak role, and role is table");
>    }
>  
> +  if (mContent->Tag() != nsGkAtoms::table)
> +    RETURN_LAYOUT_ANSWER(true, "html table without table tag");

maybe: table built by CSS display: table style?

::: accessible/tests/mochitest/table/test_layoutguess.html
@@ +100,5 @@
>  
> +      // css table with non-table tag
> +testAttrs("table23", attr, true);
> +testAttrs("table24", attr, true);
> +testAttrs("table24", attr, true);

wrong indentation

@@ +459,5 @@
> +
> +                                                                                                                    <!-- third css table with non-table tags -->
> +                                                                                                                                  <div id="table25" style="display:table">
> +                                                                                                                                          <div style="display:table-cell">Single-cell table</div>
> +                                                                                                                                              </div>

wrong indentation
btw, why do you need 3 tables instead one, i.e. what code paths you are checking?
Attachment #572407 - Flags: review?(surkov.alexander)

Updated

5 years ago
Assignee: oussamabadr → trev.saunders
Component: Disability Access → Developer Tools: Console
Target Milestone: --- → Firefox 10

Comment 33

5 years ago
Comment on attachment 572407 [details] [diff] [review]
patch

r=me with comments addressed
Attachment #572407 - Flags: review+

Updated

5 years ago
Target Milestone: Firefox 10 → ---

Comment 34

5 years ago
Created attachment 577215 [details] [diff] [review]
patch2 [for check in]
Attachment #572407 - Attachment is obsolete: true

Comment 35

5 years ago
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c5cce62078

Comment 36

5 years ago
Congratulation Alexander for fixing it...!!!
Even if busy, I don't hesitate to look around.

Comment 37

5 years ago
(In reply to Ouss. BADR from comment #36)
> Congratulation Alexander for fixing it...!!!
> Even if busy, I don't hesitate to look around.

yep, we just need this bug fixed asap so since you can't be active these days then I decided to fix nits and land Trevor's patch.
Component: Developer Tools: Console → Disability Access

Updated

5 years ago
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
https://hg.mozilla.org/mozilla-central/rev/c9c5cce62078
Status: NEW → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Updated

5 years ago
Flags: in-testsuite+
OS: Windows Vista → All
Hardware: x86 → All
You need to log in before you can comment on or make changes to this bug.