Closed Bug 424161 Opened 12 years ago Closed 12 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: MarcoZ, Assigned: surkov)

References

Details

(Keywords: access, regression)

Attachments

(1 file, 3 obsolete files)

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.
Flags: blocking1.9?
(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"?
I think patch from bug 418043 should help here. We didn't put it because the problem has gone then.
Evan's approach to fire EVENT_DOM_SIGNIFICANT_CHANGE doens't work here.
Attached patch bug418043patch (obsolete) — Splinter Review
patch from bug 418043 doens't work as well for me
Marco, when you get these errors then do you run the test separately or do you run all tests?
(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.
(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.
+'ing w/ P2 pending investigation.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
(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?
(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?
(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.
(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.
Surkov, any further ideas on this one? Could this also in some way be related to bug 424656?
Attached patch wip (obsolete) — Splinter Review
Attached patch patch (obsolete) — Splinter Review
Attachment #314507 - Attachment is obsolete: true
Attachment #314533 - Flags: review?(marco.zehe)
Comment on attachment 314533 [details] [diff] [review]
patch

R=me for the a11y part. Works for me now, thanks!
Attachment #314533 - Flags: review?(marco.zehe) → review+
Attachment #314533 - Flags: superreview?(jonas)
Attachment #314533 - Flags: review?(Olli.Pettay)
Is it possible that mRowCount is 0. Is that case handled properly?
Attachment #314533 - Flags: superreview?(jonas) → superreview+
Attached patch patch2Splinter Review
Attachment #310910 - Attachment is obsolete: true
Attachment #314533 - Attachment is obsolete: true
Attachment #314773 - Flags: review?(Olli.Pettay)
Attachment #314533 - Flags: review?(Olli.Pettay)
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;
Attachment #314773 - Flags: review?(Olli.Pettay) → review+
Attachment #314773 - Flags: approval1.9?
This bug blocks further test development in certain areas.
Comment on attachment 314773 [details] [diff] [review]
patch2

a1.9=beltzner
Attachment #314773 - Flags: approval1.9? → approval1.9+
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.