481617 - leaks of test_getChildAtPoint (enable this test)

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

see bug 481587 for description.

I can't reproduce failures of test_getChildAtPoint tests on my linux (neither running this test separately or all together). I improved comments to have a chance to understand error if something will go wrong.

Leaks are fixed.

Comment 1

[Marco Zehe (:MarcoZ)]

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

r=me. Thanks for the better message!

Comment 2

[alexander :surkov (:asurkov)]

checked in http://hg.mozilla.org/mozilla-central/rev/7b7c24f7684a

Comment 3

[alexander :surkov (:asurkov)]

tests fails on linux http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1236258672.1236262193.5183.gz

*** 87 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_childAtPoint.xul | Wrong deepest child accessible [ 'col2' , role: columnheader] at the point (157, 1) of accessible [ 'tree' ] - got [xpconnect wrapped nsIAccessible], expected [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessNode)]

Comment 4

[Boris Zbarsky [:bzbarsky]]

That looks like the test is just bogus.  Expecting particular interfaces on the object is silly, since the interfaces listed depend on who's QIed it to what.

Comment 5

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

(In reply to comment #4)
> That looks like the test is just bogus.  Expecting particular interfaces on the
> object is silly, since the interfaces listed depend on who's QIed it to what.

That must be coming from this test:
is(childAcc, actualChildAcc, msg);

I guess is() tests that the QIed interfaces are the same?

Comment 6

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

Or maybe if the pointers are not the same, that's how it reports it.

Comment 7

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

According to the log the second test fails:
testChildAtPoint(tree, x, y, true, "col1");

Let me see if I understand this. It looks like the x and y are the top left coordinates of the treecols.boxObject (relative to the document). So this test is expecting the top left coords of the treecols, to be a point contained by "col1". Maybe this is a bad assumption?

Perhaps the second test should use the x and y of the col1 boxObject? So in code-speak:

+      x = getNode("col1").boxObject.x;
+      y = getNode("col1").boxObject.y;
      testChildAtPoint(tree, x, y, true, "col1");

Thoughts?

Comment 8

[alexander :surkov (:asurkov)]

I filed bug 481721 to have a deal with failures there. Let's move discussion there please.

Comment 9

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

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

a191=beltzner

Comment 10

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

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

Actually, unapproved. :rs points out that this comes with an intermittently failing test, we can't take that on branch until it's fixed.

Comment 11

[alexander :surkov (:asurkov)]

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

reasking approval for the first part of the bug, it fixes memory leak and it's important. So we need to put this fix on the branch and leave this tests disabled.

Comment 12

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

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

a191=beltzner for the memory leak fix, do NOT land the test that fails, please, but make sure we have a follow up bug to fix that test?

Comment 13

[alexander :surkov (:asurkov)]

pushed on mozilla 1.9.1 - http://hg.mozilla.org/releases/mozilla-1.9.1/rev/127cf718d0af

(In reply to comment #12)
> (From update of attachment 365635 [details] [diff] [review])
> a191=beltzner for the memory leak fix, do NOT land the test that fails, please,
> but make sure we have a follow up bug to fix that test?

I landed memory leak fix only, bug bug 481721 is about test fails.

Comment 14

[Marco Zehe (:MarcoZ)]

Verified this landed properly on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090422 Shiretoko/3.5b4pre (.NET CLR 3.5.30729)