Intermittent failure in test_bug546995-4.html

RESOLVED INVALID

Status

()

defect
RESOLVED INVALID
9 years ago
7 years ago

People

(Reporter: Ehsan, Assigned: mounir)

Tracking

({intermittent-failure})

Trunk
mozilla2.0b9
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

9 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1274303698.1274305303.12858.gz
Rev3 Fedora 12 mozilla-central debug test mochitests-1/5 on 2010/05/19 14:14:58

41807 INFO Running /tests/content/html/content/test/test_bug546995-4.html...
++DOMWINDOW == 16 (0xe17b9ec) [serial = 1006] [outer = 0xb0242a8]
pldhash: for the table at address 0xe828370, the given entrySize of 48 probably favors chaining over double hashing.
++DOCSHELL 0xe828308 == 8
++DOMWINDOW == 17 (0xc96cd54) [serial = 1007] [outer = (nil)]
WARNING: Subdocument container has no frame: file /builds/slave/mozilla-central-linux-debug/build/layout/base/nsDocumentViewer.cpp, line 2342
++DOMWINDOW == 18 (0xc205a04) [serial = 1008] [outer = 0xc96cd20]
41808 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug546995-4.html | The iframe should have the focus - got [object HTMLBodyElement @ 0xc35df58 (native @ 0xc6c6528)], expected [object HTMLIFrameElement @ 0xe642fd0 (native @ 0xc468c38)]
41809 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug546995-4.html | The focus should be able to leave a sub-document - Didn't expect [object HTMLInputElement @ 0xcedd118 (native @ 0xe761ca0)], but got it.
41810 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug546995-4.html | The iframe should have the focus - got [object HTMLInputElement @ 0xcedd118 (native @ 0xe761ca0)], expected [object HTMLIFrameElement @ 0xe642fd0 (native @ 0xc468c38)]
Reporter

Comment 1

9 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1274305684.1274307148.20753.gz
WINNT 5.2 mozilla-central debug test mochitests-1/5 on 2010/05/19 14:48:04
Reporter

Comment 2

9 years ago
Actually, this test is wrong.  Focus is async, so by the time .focus() returns, you cannot be sure that the element is really focused.
Reporter

Comment 3

9 years ago
Hmm, I tried adding a focus event handler to the iframe before calling iframe.focus, but the event listener is never triggered.  Weird...

Comment 4

9 years ago
(In reply to comment #2)
> Actually, this test is wrong.  Focus is async, so by the time .focus() returns,
> you cannot be sure that the element is really focused.

Focusing is synchronous. Raising the window can be asynchronous. The check for document.activeElement shouldn't depend on the window being raised.
Reporter

Comment 5

9 years ago
(In reply to comment #4)
> (In reply to comment #2)
> > Actually, this test is wrong.  Focus is async, so by the time .focus() returns,
> > you cannot be sure that the element is really focused.
> 
> Focusing is synchronous. Raising the window can be asynchronous. The check for
> document.activeElement shouldn't depend on the window being raised.

Oh, I stand corrected.  So, I have no idea what's going wrong here.  Neil, could this be actually exposing a focus bug?
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 13

9 years ago
Posted patch Patch v1 (obsolete) — Splinter Review
The test is always passing on my system so I think the best we can do is to change the way the iframe is focused and see if the randomness disappears.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #446559 - Flags: review?(ehsan)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 17

9 years ago
Comment on attachment 446559 [details] [diff] [review]
Patch v1

>diff --git a/content/html/content/test/test_bug546995-4.html b/content/html/content/test/test_bug546995-4.html
>-<iframe id="iframe"></iframe>
>-<script type="application/javascript">
>-  var iframe = document.getElementById('iframe');
>-  iframe.focus();
>-  is(document.activeElement, iframe, "The iframe should have the focus");
>-</script>
>+<iframe id="iframe" onload='focus();'></iframe>

Could you change it to this.focus() to make it more clear what this does?

> function runTests()
> {
>   isnot(document.activeElement, document.getElementById('i'),
>         "The focus should not be able to leave a sub-document");
>-  is(document.activeElement, iframe, "The iframe should have the focus");
>+  is(document.activeElement, document.getElementById('iframe'),
>+     "The iframe should have the focus");

We actually don't need the isnot check here, do we?
Attachment #446559 - Flags: review?(ehsan) → review+
Reporter

Comment 18

9 years ago
Comment on attachment 446559 [details] [diff] [review]
Patch v1

Oh, wait.  Doesn't this change the meaning of the test?  What makes us sure that the autofocus related code is triggered *after* the iframe's load event?

Can't you dynamically inject the autofocus element into the document in the load event listener for the iframe?
Attachment #446559 - Flags: review+ → review-
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 20

9 years ago
Posted patch Patch v2 (obsolete) — Splinter Review
I've updated the patch to create the input element when the frame is loaded.
I prefer to keep both test even if one should be enough: it makes clearer i want input element no focused and the frame focused. I don't think having two tests instead of one is bad for anything ;)
Attachment #446559 - Attachment is obsolete: true
Attachment #446699 - Flags: review?(ehsan)
Reporter

Comment 21

9 years ago
Comment on attachment 446699 [details] [diff] [review]
Patch v2

This looks good.  But I think now you can remove the 2 second timeout here as well.  Let me explain.

We fire an autofocus event to the main thread when an element with the autofocus attribute is being bound to tree (which happens as part of appendChild call) <http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsGenericHTMLElement.cpp#2456>.  We have the executeSoon function in SimpleTest which dispatches an event to the main thread as well <http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#357>.  So, if you call executeSoon after appendChild to run runTests, you're guaranteed to execute the runTests function _after_ the autofocus event has fired, so there's no need to wait 2 seconds.

r=me with that change.
Attachment #446699 - Flags: review?(ehsan) → review+
Assignee

Comment 22

9 years ago
r=ehsan
Attachment #446699 - Attachment is obsolete: true
Assignee

Updated

9 years ago
Keywords: checkin-needed
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 24

9 years ago
Possible fix landed as:

http://hg.mozilla.org/mozilla-central/rev/5bf3cd58717d

Waiting to see if more failures are reported here.
Keywords: checkin-needed
Reporter

Updated

9 years ago
Attachment #446730 - Attachment description: Patch v3 → Patch v3 [landed on mozilla-central]
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 26

9 years ago
It looks like there has been no errors since 24 hours. Let's consider this bug as fixed.
Do not hesitate to reopen you still get a random orange.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot)
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9.3a5
Reporter

Comment 28

9 years ago
(In reply to comment #27)
> Olli.Pettay%gmail.com
> http://tinderbox.mozilla.org/showlog.cgi?log=Electrolysis/1274778789.1274781523.30108.gz
> WINNT 5.2 electrolysis debug test mochitests-1/5, Started 02:13, finished
> 02:59, took 46mins
> 
> s: win32-slave56
> 41818 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/html/content/test/test_bug546995-4.html |
> [SimpleTest/SimpleTest.js, window.onerror] An error occurred - onFrameLoad is
> not defined at
> http://mochi.test:8888/tests/content/html/content/test/test_bug546995-4.html:1
> 41819 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/html/content/test/test_bug546995-4.html | Test timed out.

This is in fact another problem, similar to bug 567014.  I filed this as bug 568120.
Comment hidden (Legacy TBPL/Treeherder Robot)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 34

9 years ago
Three months with nothing before this... is that usual for an intermittent failure?
Reporter

Comment 35

9 years ago
(In reply to comment #34)
> Three months with nothing before this... is that usual for an intermittent
> failure?

It's not, but the original problem doesn't seem to be completely fixed anyway.
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 46

9 years ago
content/html/content/test/test_bug546995-4.html do not exist anymore. It has been translated into a reftest because autofocus behavior has changed for security reasons (bug 601030).

Marking invalid.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → INVALID
Assignee

Updated

9 years ago
Target Milestone: mozilla1.9.3a5 → mozilla2.0b9
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.