575595 - add GetRowAndColumnIndicesAt to make IAccessibleTable::get_rowColumnExtentsAtIndex faster

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

It should allow make the table test (bug 570995) faster in two times because of reduction of nsOuterTableFrame::GetRowAndColumnByIndex() calls number which is bottle neck now.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

try server build - http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-9ac34b5c7b36/

Comment 3

[Marco Zehe (:MarcoZ)]

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

r=me for the tests and the functionality. This improves perf of the table mutation test from the 35400/35350 MS of try-server build of bug 575576 to 24409/24386 MS with this try-server build. Additionally, initial loading time of the page with NVDA is vastly improved.

Comment 4

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

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

r=me! Only nits:

>+++ b/accessible/src/msaa/CAccessibleTable.cpp

I'm not sure the cosmetic changes are worth the loss in history/annotation -- but I'll leave it up to you to keep them or not.

>+nsXULTreeGridAccessible::GetRowAndColumnIndicesAt(PRInt32 aCellIndex,
>+                                                  PRInt32* aRowIndex,
>+                                                  PRInt32* aColumnIndex)
>+{
>+  NS_ENSURE_ARG_POINTER(aRowIndex);
>+  *aRowIndex = -1;
>+  NS_ENSURE_ARG_POINTER(aColumnIndex);
>+  *aColumnIndex = -1;

Nit: this is fine or you might prefer:

NS_ENSURE_ARG_POINTER(aRowIndex);
NS_ENSURE_ARG_POINTER(aColumnIndex);
*aRowIndex = *aColumnIndex = -1;


>+          ok(false, id + ": can't get row and column indexes for cell index " + idx + "," + e);

Nit: "indices"

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> (From update of attachment 454852 [details] [diff] [review])
> r=me! Only nits:
> 
> >+++ b/accessible/src/msaa/CAccessibleTable.cpp
> 
> I'm not sure the cosmetic changes are worth the loss in history/annotation --
> but I'll leave it up to you to keep them or not.

That's always a choice: nicer code vs cleaner history. I don't have an answer. Here I would prefer to keep.

> NS_ENSURE_ARG_POINTER(aRowIndex);
> NS_ENSURE_ARG_POINTER(aColumnIndex);
> *aRowIndex = *aColumnIndex = -1;

this shorter and nicer but not correct though nobody cares in the end I guess.

Comment 6

[alexander :surkov (:asurkov)]

landed on 2.0 (1.9.3) - http://hg.mozilla.org/mozilla-central/rev/5425902639a5