409009 - Table cells should have a table in their ancestry

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Attachment: test case

Step to reproduce:

1. Load the test case and view the hierarchy in Accerciser

Expected Results:  

EITHER the text and links on the page would not be exposed as table cells because they're not in an HTML table, OR a proper table should be in the ancestry of the cells.

Actual Results:

Because the display: table-cell; style has been applied, these items are exposed as table cells, but a table is not in their ancestry, and calls that we expect to be able to make to the table interface tend to fail.

Requesting this be considered as a FF3 commitment.  Thanks!

Comment 1

[alexander :surkov (:asurkov)]

It's rather wrong css usage than real bug, but since Mozilla renders this fine and creates table cell frame so we should do something here. We need to get an idea how table layout code works to take the decision I guess. Though I don't catch why it's needed to be in FF3 timeframe?

Comment 2

[Joanmarie Diggs]

Thanks Alexander!

It's needed for the FF3 timeframe because I am currently working on moving Orca away from using extents to obtain line contents, instead using getTextAtOffset().  We're seeing a great improvement in performance, so it's definitely worth it.  However, the problem of presenting the "current line" to the user is that the object at the beginning of the line might be a link or a table cell or something else that getTextAtOffset() won't provide.  In those instances we have to do a bit of work figure out what else is on the line and get its text.

w.r.t. this bug:  The logical "bit of work" to do when the first thing is a table cell is to examine the table to see if there are additional cells on the right with text on the current line.  We can't do this given the bogus CSS and subsequent rendering because we're in a table cell that's not in a table. So we're forced to revert to getting extents. :-(

Thanks for considering it!

Comment 3

[alexander :surkov (:asurkov)]

Sorry, I still do not get.

Why this syntax is preferable

<div>
  <div style="display: table-cell">Links:&nbsp;</div>
  <div style="display: table-cell">
    <a href="foo">foo</a>
    <a href="bar">bar</a>
  </div>
</div>

than
<div style="display: table">
  <div style="display: table-cell">Links:&nbsp;</div>
  <div style="display: table-cell">
    <a href="foo">foo</a>
    <a href="bar">bar</a>
  </div>
</div>
or something like this where you use proper table display styles?

Is that a real usecase you want to support or?

Comment 4

[Joanmarie Diggs]

I'm not sure I follow your question.  I'm not saying that the above syntax is preferable.  We came across it on an actual site. 

When we come across an object which is exposed as ROLE_TABLE_CELL, we expect to be able to query an ancestor which implements the table interface.   After all, isn't the definition of a table cell a cell inside a table? :-)

If an object is not really a table cell (i.e. a cell within a table), perhaps the best thing to do would not be to expose the object as such....

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> I'm not sure I follow your question.  I'm not saying that the above syntax is
> preferable.  We came across it on an actual site. 
> 

Ah, ok, that I was asked. You just find some web page using css style in that way and you get your approach doesn't work there, right?

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch

ignore cell and etc accessibles if there is no parent table accessible

Comment 7

[Evan Yan]

Comment on attachment 294534 [details] [diff] [review]
patch

>+      if (frame->GetType() == nsAccessibilityAtoms::tableRowGroupFrame ||
>+          frame->GetType() == nsAccessibilityAtoms::tableRowFrame ||
>+          frame->GetType() == nsAccessibilityAtoms::tableCellFrame) {
>+
How about TableCaptionFrame?

>-          if (tableContent->Tag() == nsAccessibilityAtoms::table) {
>+          nsIFrame *tableFrame = aPresShell->GetPrimaryFrameFor(tableContent);
>+          if (tableFrame &&
>+              tableFrame->GetType() == nsAccessibilityAtoms::tableOuterFrame) {

This change will make the loop go father than we want.

for example:

<table>
  <tr><td>
    <table style="display:block">
      <tr><td>some text</td></tr>
    </table>
  </td></tr>
<table>

Comment 8

[alexander :surkov (:asurkov)]

Attachment: patch2

better?

Comment 9

[Evan Yan]

Comment on attachment 294814 [details] [diff] [review]
patch2

r=me

>+          if (tableFrame &&
>+              tableFrame->GetType() == nsAccessibilityAtoms::tableOuterFrame) {
>             // Table that we're a descendant of is not styled as a table,
>             // and has no table accessible for an ancestor, or
>             // table that we're a descendant of is presentational
> 
I feel the comment was kind of not explaining the code correctly. Would you mind fix it also when commit?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #9)

> I feel the comment was kind of not explaining the code correctly. Would you
> mind fix it also when commit?
> 

sure, I will

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 294814 [details] [diff] [review]
patch2

a=beltzner for 1.9

Comment 12

[Joanmarie Diggs]

What's the status of this?

Comment 13

[alexander :surkov (:asurkov)]

sorry for delay. I checked in the patch without uncorrect comment, because it sounds top comment explains good enough what we do.