leaks of test_getChildAtPoint (enable this test)

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

({verified1.9.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Created attachment 365635 [details] [diff] [review]
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.
Attachment #365635 - Flags: review?(marco.zehe)
(Assignee)

Updated

10 years ago
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED

Comment 1

10 years ago
Comment on attachment 365635 [details] [diff] [review]
patch

r=me. Thanks for the better message!
Attachment #365635 - Flags: review?(marco.zehe) → review+
(Assignee)

Updated

10 years ago
Depends on: 481623
(Assignee)

Updated

10 years ago
Blocks: 481623
No longer depends on: 481623
(Assignee)

Updated

10 years ago
Severity: normal → major
(Assignee)

Comment 2

10 years ago
checked in http://hg.mozilla.org/mozilla-central/rev/7b7c24f7684a
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Attachment #365635 - Flags: approval1.9.1?
(Assignee)

Comment 3

10 years ago
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)]
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.
(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?
Or maybe if the pointers are not the same, that's how it reports it.
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?
(Assignee)

Comment 8

10 years ago
I filed bug 481721 to have a deal with failures there. Let's move discussion there please.
Comment on attachment 365635 [details] [diff] [review]
patch

a191=beltzner
Attachment #365635 - Flags: approval1.9.1? → approval1.9.1+
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.
Attachment #365635 - Flags: approval1.9.1+ → approval1.9.1-
(Assignee)

Comment 11

10 years ago
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.
Attachment #365635 - Flags: approval1.9.1- → approval1.9.1?
Attachment #365635 - Flags: approval1.9.1? → approval1.9.1+
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?
(Assignee)

Comment 13

10 years ago
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.
Keywords: fixed1.9.1

Comment 14

10 years ago
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)
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.