404881 - Accessible tree for role=log table not updated for td's without aria-live role.

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[:Gijs (he/him)]

Attachment: Testcase

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112106 Minefield/3.0b2pre

STR:

1. Open testcase
2. Open accexplorer, select testcase, check tree for the table in there.
3. Whack "plain msg" button a bunch of times.
4. refresh tree in accexplorer

ER: all messages show up
AR: none of them do

5. whack "assertive msg" button
6. Refresh tree in accexplorer, and wonder why suddenly all those nodes are showing up.

(it is also a bit strange that for the assertive objects a different accessible seems to be used, but I'm not entirely sure what's up with that or what kind of accessible it is)


Thanks to MarcoZ for telling me about this - it makes ChatZilla pretty much unusable with Jaws (as messages won't show up until someone uses your name...).

Comment 1

[:Gijs (he/him)]

Interestingly, it does not look like this bug is exhibited for a <div> with separate <div>'s added as messages.

Comment 2

[alexander :surkov (:asurkov)]

Gijs, does it work if table hasn't ARIA role?

Comment 3

[:Gijs (he/him)]

(In reply to comment #2)
> Gijs, does it work if table hasn't ARIA role?
> 

If I remove "role='log'" and leave aria-live=polite it still doesn't work.
If I remove "role='log'" as well as "aria-live='polite'" it still doesn't work.

Comment 4

[Marco Zehe (:MarcoZ)]

This is a regression from Firefox 2.0. In Firefox 2, the content gets updated appropriately. I do not even have to use JAWS Refresh, the new content is visible to me as soon as I hit either button.

I'll try to find a regression range.

Comment 5

[Marco Zehe (:MarcoZ)]

The regression range is:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-06-30+02%3A00&maxdate=2006-07-01+05%3A00&cvsroot=%2Fcvsroot

So, this goes way back to July 1st, 2006.

Comment 6

[:Gijs (he/him)]

Thanks a lot Marco!

Looks like bug 339565 or (more likely) bug 342979. (not bug 339237 as it was backed out)

I do wonder why the update thing is broken for tables without role=log...

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

My gut reaction: I'm guessing Aaron might know. Perhaps a performance decision not to create nodes if the message is polite?  Have you tried non table elements?

Comment 8

[Marco Zehe (:MarcoZ)]

Found that this is also not working on Linux. Changing platform and OS accordingly.

Comment 9

[alexander :surkov (:asurkov)]

Sorry, what kind of events you don't see?

Comment 10

[Marco Zehe (:MarcoZ)]

(In reply to comment #9)
> Sorry, what kind of events you don't see?

We don't see the content getting updated once the first of the buttons is pressed. New text appears on the screen, but the accessible tree under both Windows and Linux is not being updated at all. It is as if the new text never was written.

Comment 11

[alexander :surkov (:asurkov)]

what exactly events should be fired?

Comment 12

[Marco Zehe (:MarcoZ)]

(In reply to comment #11)
> what exactly events should be fired?

First, the accessible module should be notified that the text is being inserted into the document. Load the test case and try it out: If you click the first button, the text gets inserted into the document, and appears on the screen. But it does not appear in the accessibility tree! It is not there. As if it was never inserted.

Comment 13

[alexander :surkov (:asurkov)]

I see 'DOM node create' events which should be mapped into show events on platforms when I click on any button. Do you mean we should fire 'text inserted/removed' events as well? Can you think of reason why accessible tree is no updated because I see correct accessible tree on cross platform side (via DOMi)?

Comment 14

[Marco Zehe (:MarcoZ)]

Attachment: Modified testcase

This testcase adds an aria-live="polite" attribute to each row that gets created which is not assertive. This also causes the accessible information to be updated each time.

I also noticed two other things:
1. Each row that gets appended by JavaScript is a child of the Table node. However, the row that is hardcoded into the HTML is created under a TBODY element. Changing the JavaScript to also insert the rows under the TBODY did not fix the problem.
2. Without aria-live="polite", each row would only be created as a single table cell once the a11y info was updated. However, DomI shows each one as a row properly.

So the question is: If aria-live="polite" is already set for the table, why are the accessibles not updated when inserted, and why are they only inserted as cells, not as complete rows?

Comment 15

[Marco Zehe (:MarcoZ)]

Additionally, I am no longer able to repro this on Linux. Setting the bug back to being Windows-specific, which it definitely still is.

Comment 16

[:Gijs (he/him)]

Surkov:
Re: "what events should be fired" - I don't *think* this is about events at all. From my limited understanding of the issue, if you tell accexplorer to update its view, it will do that no matter what events it received. So if I look at the table with accexplorer before adding rows, add a row (without an aria-live attribute) and then look again (ie, explicitly tell accexplorer to update the view), then I still don't see this new row, while I should. As far as I can tell that has nothing to do with what events we do or do not fire.

Comment 17

[:Gijs (he/him)]

Any chance of an update here, guys? :-)

Comment 18

[Aaron Leventhal]

I am not seeing a problem with the testcase.

Actually I think we may have broken something else that made this work: tbody, td and tr objects are not supposed to get accessibility objects when their ancestor table has a role unless they have an interesting property that needs an object in order to be exposed (such as live). The reason those descendant table items aren't normally exposed in a non-table <table> is that we don't want to confuse ATs by showing them a ROW or CELL that aren't actually inside a ROLE_TABLE.

Something has changed recently though, and we are exposing ROLE_CELLs even though the ancestor table has a role. That's probably "accidentally" fixed this. I suspect it might be one of Surkov's recent table fixes.

What I think we should do is automatically expose row objects as generic accessibles with something like ROLE_PARAGRAPH or ROLE_SECTION in <table>'s that have an ARIA role. And we should fix the regression where table cells are showing up in that case even when there is nothing special about them.  If there is something interesting they should still not be ROLE_CELL.

Comment 19

[Aaron Leventhal]

Attachment: Clean up table-related accessible descendant rules

1) Only create table-related accessibles if inside ROLE_TABLE ancestor
2) Don't create table-related accessibles at all, if table has role="presentation"
3) Create generic accessibles in table w/ any other ARIA role, so that role attribute || tag name is exposed
4) If an ARIA role is present on an accesible, never use the role from the implementation -- use ROLE_NOTHING so that AT sees ARIA role || tag name

This ensures the right accessibles are created inside presentation tables, log tables, true data tables and any other kind of table.

Comment 20

[alexander :surkov (:asurkov)]

could you please put -w patch as well?

Comment 21

[Aaron Leventhal]

Attachment: Without whitespace chagges

Comment 22

[alexander :surkov (:asurkov)]

Comment on attachment 299367 [details] [diff] [review]
Clean up table-related accessible descendant rules


>+          if (nsAccessible::Role(tableAccessible) != nsIAccessibleRole::ROLE_TABLE)
>+            roleMapEntry = &nsARIAMap::gLandmarkRoleMap; // Not in table: override role

please mention that roleMapEntry is always nsnull here otherwise it makes think you want to redefine roleMapEntry

>+          break;
>+        }
>+        else if (tableFrame->GetType() == nsAccessibilityAtoms::tableCellFrame ||
>+                 tableContent->Tag() == nsAccessibilityAtoms::table {

why do we need to wide the condition tableContent->Tag() == nsAccessibilityAtoms::table?

>+          // Stop before we are fooled by any additional table ancestors
>+          roleMapEntry = &nsARIAMap::gLandmarkRoleMap; // Not in table: override role

the same

>-        else if (content->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::aria_secret,
>+        else if (content->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::aria_haspopup,
>                                       nsAccessibilityAtoms::_true, eCaseMatters)) {

seems from antother bug, please do not mix them.

> 
>-    if (*aRole != nsIAccessibleRole::ROLE_NOTHING) {
>-      return NS_OK;
>-    }
>+    return NS_OK;

can you describe this change?

Comment 23

[Aaron Leventhal]

Attachment: Comments and assertions added, and removed Tag() == table check since it' s not really useful and arguably incorrect

Comment 24

[Aaron Leventhal]

Attachment: Without whitespace changes

Comment 25

[alexander :surkov (:asurkov)]

if now we do not create accessible or create hyper text accessible then your patch will create tag/frame based accessible for example when cell is not inside table. Is it desired?

Comment 26

[Aaron Leventhal]

I think it is valuable to create an accessible anyway for the layout object because the cell controls the line wrapping and it can be a multi line cell. Therefore treating it as a span in a line makes no sense.

Comment 27

[alexander :surkov (:asurkov)]

(In reply to comment #26)
> I think it is valuable to create an accessible anyway for the layout object
> because the cell controls the line wrapping and it can be a multi line cell.
> Therefore treating it as a span in a line makes no sense.
> 

I'm not sure I completely follow you because I don't know how can accessible created by markup/frame with landscape role be used, I mean its specific interfaces can't be used. And therefore it should be enough to create generic accessible. Though I don't see any badness to create accessible by markup/frame actually.

Comment 28

[Aaron Leventhal]

Landmark role just means the xml-roles or tag attribute needs to be checked by the AT.

My idea is that if the table frames are being used, the table interface can still be suported. So if someone creates something such as a log by using a table, the AT can still use the table interfaces to grant table navigation to the log if it wants to.

Do you think that's okay, or what's your alternate suggestion?

Comment 29

[alexander :surkov (:asurkov)]

Comment on attachment 299449 [details] [diff] [review]
Without whitespace changes

r=me

Comment 30

[alexander :surkov (:asurkov)]

(In reply to comment #28)
> Landmark role just means the xml-roles or tag attribute needs to be checked by
> the AT.

in our case xml-roles attribute isn't used, only tag attribute if table wasn't styled, so i'm not sure how it can be used.

> Do you think that's okay, or what's your alternate suggestion?
> 

I think it's ok because it shouldn't break anything.

I haven't actually.

Comment 31

[Damon Sicore (:damons)]

Comment on attachment 299449 [details] [diff] [review]
Without whitespace changes

a1.9+=damons

Comment 32

[Marco Zehe (:MarcoZ)]

Today's Trunk build no longer exposes the ChatZilla output window as a table, but now, the updating problem also returned. Unless someone pings me directly, the Accessible Info is not updated with incoming messages. Once someone pings me, all the updates happen. Reopening.

Comment 33

[Marco Zehe (:MarcoZ)]

False alarm, sorry for the spam! The stuff does update. I was just irritated by the fact that now, because the ChatZilla output is no longer being exposed as a data table, much text is being run together without line breaks, and the text I had typed actually appeared on the same last line that I had previously seen.
I'll file a separate bug on this new issue and reset this bug to resolved FIXED.

Comment 34

[Marco Zehe (:MarcoZ)]

Verified using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3

Comment 35

[Evan Yan]

With this bug's testcase, I see this accessible hierarchy:

table
-- table cell
-- hypertext
   -- table cell

I think it's not correct that the second table cell is a child of an hypertext.

Comment 36

[Marco Zehe (:MarcoZ)]

Evan, I don't see anything wrong with the table hierarchy in Dom Inspector's Accessible Tree. Also all the Mochitests Bernd put into bug 415730 seem to indicate that things are OK with tables.

Comment 37

[Evan Yan]

Marco, the problem is a child of table accessible doesn't have ROLE_TABLE_CELL, and the parent of a table cell accessible doesn't have ROLE_TABLE.

Comment 38

[Aaron Leventhal]

Evan, it could be the <thead>, <tbody> or <tfoot> are getting accessibles on Linux now. We only used to allow that on IA2.

Comment 39

[Evan Yan]

bug 419811 created for the html table accessible issue.