501635 - nsHTMLTableAccessible::GetSelectedCells contains index duplicates for spanned rows or columns

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

If HTML table has row or column spans and spanned row or column is selected then GetSelectedCells returns an array containing number of identical indexes equaled to extent of spans.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[Marco Zehe (:MarcoZ)]

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

r=me for the test part. Thanks!

Comment 3

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

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

r=me with questions answered:

>+      rv = tableLayout->GetCellDataAt(rowIndex, columnIndex,
>+                                      *getter_AddRefs(domElement),
>+                                      startRowIndex, startColIndex,
>+                                      rowSpan, colSpan,
>+                                      actualRowSpan, actualColSpan,
>+                                      isSelected);
> 
>-      if (state)
>+      if (NS_SUCCEEDED(rv) && startRowIndex == rowIndex &&
>+          startColIndex == columnIndex && isSelected) {

Can you remind me when startColIndex != columnIndex?

>   // isCellSelected test
>   for (var rowIdx = 0; rowIdx < rowsCount; rowIdx++) {
>     for (var colIdx = 0; colIdx < colsCount; colIdx++) {
>+      if (aCellsArray[rowIdx][colIdx] == undefined)
>+        continue;

When does this happen?


>   is(accTable.selectedColumnsCount, 0, "no column should be selected");
>   ok(!accTable.isProbablyForLayout(), "table is not for layout");
>   SimpleTest.finish();
>-}
>+    }

nit: indent looks wrong?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> (From update of attachment 386263 [details] [diff] [review])

> Can you remind me when startColIndex != columnIndex?

when col or row is spanned by another row or col

> When does this happen?

when we pass undefined there :) see html test file.

> 
> nit: indent looks wrong?

right, I just saved existing indents to avoid hg changes, later when I fix all related bugs then I'll replace them on more suitable testing.

Comment 5

[Marco Zehe (:MarcoZ)]

Pushed on Surkov's behalf in changeset:
http://hg.mozilla.org/mozilla-central/rev/6d2ad89d3afa