Closed Bug 566983 Opened 15 years ago Closed 15 years ago

Intermittent failure in test_bug546995-4.html

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID
Milestone:
mozilla2.0b9

People

(Reporter: ehsan.akhgari, Assigned: mounir)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

Patch v3 [landed on mozilla-central]
2.37 KB, patch
Details | Diff | Splinter Review
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)]
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
Actually, this test is wrong. Focus is async, so by the time .focus() returns, you cannot be sure that the element is really focused.
Hmm, I tried adding a focus event handler to the iframe before calling iframe.focus, but the event listener is never triggered. Weird...
(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.
(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?
Attached 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 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+
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-
Attached 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)
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+
r=ehsan
Attachment #446699 - Attachment is obsolete: true
Keywords: checkin-needed
Possible fix landed as: http://hg.mozilla.org/mozilla-central/rev/5bf3cd58717d Waiting to see if more failures are reported here.
Keywords: checkin-needed
Attachment #446730 - Attachment description: Patch v3 → Patch v3 [landed on mozilla-central]
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: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9.3a5
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Three months with nothing before this... is that usual for an intermittent failure?
(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.
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: 15 years ago → 15 years ago
Resolution: --- → INVALID
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.

Attachment

General

Creator:
Created:
Updated:
Size: