424161 - Tree invalidation screwed up badly, getting 6 failures on Mochitest for bug 368835.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[Marco Zehe (:MarcoZ)]

The failures are:
Passed: 10
Failed: 6
Todo: 0
not ok - Wrong 'startrow' of 'treeInvalidated' event on InvalidateRow(). got 0, expected null
not ok - Wrong 'endrow' of 'treeInvalidated' event on InvalidateRow(). got 0, expected null
not ok - Wrong 'startrow' of 'treeInvalidated' event on InvalidateColumn(). got 3, expected 1
not ok - Wrong 'endrow' of 'treeInvalidated' event on InvalidateColumn(). got 3, expected 1
not ok - Wrong 'startcolumn' of 'treeInvalidated' event on InvalidateColumn(). got 0, expected null
not ok - Wrong 'endcolumn' of 'treeInvalidated' event on InvalidateColumn(). got 0, expected null

In addition, Mike Pedersen from the Orca team reports that, after fix for bug 421922 landed and tree table accessibles are restored, the info Orca is getting is complete nonsense.

Comment 1

[alexander :surkov (:asurkov)]

(In reply to comment #0)

> In addition, Mike Pedersen from the Orca team reports that, after fix for bug
> 421922 landed and tree table accessibles are restored, the info Orca is getting
> is complete nonsense.
> 

Marco, could you give more details, what is "complete nonsense"?

Comment 2

[alexander :surkov (:asurkov)]

I think patch from bug 418043 should help here. We didn't put it because the problem has gone then.

Comment 3

[alexander :surkov (:asurkov)]

Evan's approach to fire EVENT_DOM_SIGNIFICANT_CHANGE doens't work here.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: bug418043patch

patch from bug 418043 doens't work as well for me

Comment 5

[alexander :surkov (:asurkov)]

Marco, when you get these errors then do you run the test separately or do you run all tests?

Comment 6

[Marco Zehe (:MarcoZ)]

(In reply to comment #5)
> Marco, when you get these errors then do you run the test separately or do you
> run all tests?

It doesn't matter, happens both when I run all test and when I run this test alone. Same failures each time.

Comment 7

[Marco Zehe (:MarcoZ)]

(In reply to comment #1)
> (In reply to comment #0)
> 
> > In addition, Mike Pedersen from the Orca team reports that, after fix for bug
> > 421922 landed and tree table accessibles are restored, the info Orca is getting
> > is complete nonsense.
> Marco, could you give more details, what is "complete nonsense"?

Column information being in the wrong order or from different messages (e. g. the subject from one, the author of another), or some rows appear empty.

Comment 8

[Damon Sicore (:damons)]

+'ing w/ P2 pending investigation.

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> (In reply to comment #5)
> > Marco, when you get these errors then do you run the test separately or do you
> > run all tests?
> 
> It doesn't matter, happens both when I run all test and when I run this test
> alone. Same failures each time.
> 

Marco, could you try the uploaded patch?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #7)
> (In reply to comment #1)
> > (In reply to comment #0)
> > 
> > > In addition, Mike Pedersen from the Orca team reports that, after fix for bug
> > > 421922 landed and tree table accessibles are restored, the info Orca is getting
> > > is complete nonsense.
> > Marco, could you give more details, what is "complete nonsense"?
> 
> Column information being in the wrong order or from different messages (e. g.
> the subject from one, the author of another), or some rows appear empty.
> 


Can you file new bug for this since it's another issue?

Comment 11

[Marco Zehe (:MarcoZ)]

(In reply to comment #10)
> (In reply to comment #7)
> > (In reply to comment #1)
> > > (In reply to comment #0)
> > > > In addition, Mike Pedersen from the Orca team reports that, after fix for bug
> > > > 421922 landed and tree table accessibles are restored, the info Orca is getting
> > > > is complete nonsense.
> > > Marco, could you give more details, what is "complete nonsense"?
> > Column information being in the wrong order or from different messages (e. g.
> > the subject from one, the author of another), or some rows appear empty.
> Can you file new bug for this since it's another issue?

Done, bug 424656.

Comment 12

[Marco Zehe (:MarcoZ)]

(In reply to comment #9)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Marco, when you get these errors then do you run the test separately or do you
> > > run all tests?
> > It doesn't matter, happens both when I run all test and when I run this test
> > alone. Same failures each time.
> Marco, could you try the uploaded patch?

With that patch, if I run it alone, the failed falls to 0, but when I run it with all others, I have 3 failures.

Comment 13

[Marco Zehe (:MarcoZ)]

Surkov, any further ideas on this one? Could this also in some way be related to bug 424656?

Comment 14

[alexander :surkov (:asurkov)]

Attachment: wip

Comment 15

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 16

[Marco Zehe (:MarcoZ)]

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

R=me for the a11y part. Works for me now, thanks!

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Is it possible that mRowCount is 0. Is that case handled properly?

Comment 18

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

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


>+#ifdef ACCESSIBILITY
>+  nsIPresShell *presShell = PresContext()->PresShell();
>+  if (presShell->IsAccessibilityActive()) {
>+    PRInt32 end = mRowCount > 0 ? ((mRowCount <= aEnd) ? mRowCount - 1 : aEnd) :
>+                                  0;

Maybe this way:
    PRInt32 end =
      mRowCount > 0 ? ((mRowCount <= aEnd) ? mRowCount - 1 : aEnd) : 0;

Comment 20

[Marco Zehe (:MarcoZ)]

This bug blocks further test development in certain areas.

Comment 21

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

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

a1.9=beltzner

Comment 22

[Marco Zehe (:MarcoZ)]

Checking in accessible/src/base/nsRootAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v  <--  nsRootAccessible.cpp
new revision: 1.267; previous revision: 1.266
done
Checking in accessible/tests/mochitest/test_bug368835.xul;
/cvsroot/mozilla/accessible/tests/mochitest/test_bug368835.xul,v  <--  test_bug368835.xul
new revision: 1.6; previous revision: 1.5
done
Checking in layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp,v  <--  nsTreeBodyFrame.cpp
new revision: 1.354; previous revision: 1.353
done

Checked in at Alexander's request.