Closed
Bug 725015
Opened 12 years ago
Closed 12 years ago
[SeaMonkey] permanent "dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out."
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla13
People
(Reporter: sgautherie, Assigned: sgautherie)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [perma-orange])
Attachments
(1 file, 2 obsolete files)
1.39 KB,
patch
|
mounir
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
SM 2.8: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1328566990.1328572485.22195.gz WINNT 5.2 comm-beta debug test mochitests-2/5 on 2012/02/06 14:23:10 is already failing. SM 2.9: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1328598005.1328601563.15240.gz WINNT 5.2 comm-aurora debug test mochitests-2/5 on 2012/02/06 23:00:05 is already failing. SM 2.10: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1328503421.1328505166.30593.gz Linux comm-central-trunk debug test mochitests-2/5 on 2012/02/05 20:43:41 { ... 14285 INFO TEST-PASS | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | parameter screenY should be taken into account 14286 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out. SCREENSHOT: data:image/png;base64,[...] 14287 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 308908ms } NB: Screenshot simply shows the small opened window :-| Reproducible on my local Windows 2000. ***** This test needs to ensure "dom.disable_window_move_resize" is set to false! http://mxr.mozilla.org/comm-central/search?string=dom.disable_window_move_resize&case=1&find=\.js
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #595119 -
Flags: review?(mounir)
Comment 2•12 years ago
|
||
Comment on attachment 595119 [details] [diff] [review] (Av1) Add missing 'return', Set needed preference, Improve some nits Review of attachment 595119 [details] [diff] [review]: ----------------------------------------------------------------- A few of those changes are not very easy to understand. Could you give more context? ::: dom/tests/mochitest/bugs/test_resize_move_windows.html @@ +72,3 @@ > if (condition() || times == 0) { > test(); > + SimpleTest.executeSoon(next); Why? @@ +261,5 @@ > }); > }); > } > > +SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize", false]]}, runTests); I would prefer: SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize", false]]}, function() { SimpleTest.waitForFocus(function() { // }) }); @@ +269,4 @@ > if (screen.width <= 200 || screen.height <= 200) { > + todo(false, "Screen needs to be at least 200x200px to run this test."); > + SimpleTest.executeSoon(SimpleTest.finish); > + return; Why? @@ +281,5 @@ > // However, passing size/position parameters to window.open should work. > var w = window.open("data:text/html,<script>" + > "function check(next) {" + > " var is_range = function(aTest, aValue, aRange, aMsg) {" + > + " window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" + Does not seem useful. @@ +313,5 @@ > checkChangeIsDisabled(w, function() { > w.close(); > > restoreValues(); > + SimpleTest.executeSoon(SimpleTest.finish); Why?
Attachment #595119 -
Flags: review?(mounir)
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #2) > ::: dom/tests/mochitest/bugs/test_resize_move_windows.html > @@ +72,3 @@ > > if (condition() || times == 0) { > > test(); > > + SimpleTest.executeSoon(next); > > Why? executeSoon(), here and elsewhere, lets the calling callback fully end, fwiw: optional. > I would prefer: > SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize", > false]]}, function() { > SimpleTest.waitForFocus(function() { > // > }) > }); As you prefer: I will do. > @@ +269,4 @@ > > if (screen.width <= 200 || screen.height <= 200) { > > + todo(false, "Screen needs to be at least 200x200px to run this test."); > > + SimpleTest.executeSoon(SimpleTest.finish); > > + return; > > Why? (Why what?) > @@ +281,5 @@ > > // However, passing size/position parameters to window.open should work. > > var w = window.open("data:text/html,<script>" + > > "function check(next) {" + > > " var is_range = function(aTest, aValue, aRange, aMsg) {" + > > + " window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" + > > Does not seem useful. Easier to read to me: optional.
Comment 4•12 years ago
|
||
Please note that I just commited something with the wrong bug number in it: if you see a patch about category entries and observer notifications, you want bug 725016, not this bug.
Assignee | ||
Updated•12 years ago
|
Attachment #595119 -
Flags: review?(mounir)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to TinderboxPushlog Robot from comment #5) > mak77%bonardo.net > https://tbpl.mozilla.org/php/getParsedLog.php?id=9409823&tree=Firefox Not this SeaMonkey bug.
Assignee | ||
Comment 7•12 years ago
|
||
Av1, without executeSoon() and with s/200/201/g.
Attachment #595119 -
Attachment is obsolete: true
Attachment #595119 -
Flags: review?(mounir)
Attachment #598997 -
Flags: review?(mounir)
Comment 8•12 years ago
|
||
Comment on attachment 598997 [details] [diff] [review] (Av2) Add missing 'return', Set needed preference, Improve some nits Review of attachment 598997 [details] [diff] [review]: ----------------------------------------------------------------- Dropping the review request because I would like to see the fixed patch. ::: dom/tests/mochitest/bugs/test_resize_move_windows.html @@ -22,5 @@ > SimpleTest.waitForExplicitFinish(); > > var previousX, previousY, previousWidth, previousHeight; > > - no need to remove this line. @@ +67,5 @@ > * If times < 0, the event loop will be hitten as long as the condition isn't > * true or the test doesn't time out. > */ > +function hitEventLoop(condition, test, times, next) > +{ no need to change the style @@ +266,3 @@ > SimpleTest.waitForFocus(function() { > + if (screen.width < 201 || screen.height < 201) { > + todo(false, "Screen needs to be at least 201x201px to run this test."); Ok to change the todo description (I would have preferred "The screen needs to be bigger than 200px*200px to run this test.") but I don't think you need to change the condition. @@ +278,5 @@ > // However, passing size/position parameters to window.open should work. > var w = window.open("data:text/html,<script>" + > "function check(next) {" + > " var is_range = function(aTest, aValue, aRange, aMsg) {" + > + " window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" + I would prefer if you could drop that change.
Attachment #598997 -
Flags: review?(mounir)
Assignee | ||
Comment 9•12 years ago
|
||
Av2, with comment 8 suggestion(s).
Attachment #598997 -
Attachment is obsolete: true
Attachment #601274 -
Flags: review?(mounir)
Comment 10•12 years ago
|
||
Comment on attachment 601274 [details] [diff] [review] (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit [Checked in: Comments 11 and 15] Review of attachment 601274 [details] [diff] [review]: ----------------------------------------------------------------- Thanks :) r=me
Attachment #601274 -
Flags: review?(mounir) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Comment on attachment 601274 [details] [diff] [review] (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit [Checked in: Comments 11 and 15] https://hg.mozilla.org/mozilla-central/rev/30b4f99a137c Succeeded as https://tbpl.mozilla.org/?tree=Try&rev=6d1a41cb8058 [Approval Request Comment] Regression caused by (bug #): Bug 565541. User impact if declined: None, but perma-orange (timeout) on SeaMonkey. Testing completed (on m-c, etc.): Try and this comment. Risk to taking this patch (and alternatives if risky): None, test only. String changes made by this patch: None.
Attachment #601274 -
Attachment description: (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit → (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comment 11]
Attachment #601274 -
Flags: approval-mozilla-beta?
Attachment #601274 -
Flags: approval-mozilla-aurora?
Comment 12•12 years ago
|
||
Try run for 6d1a41cb8058 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=6d1a41cb8058 Results (out of 30 total builds): exception: 1 success: 27 warnings: 2 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sgautherie.bz@free.fr-6d1a41cb8058
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment 13•12 years ago
|
||
Comment on attachment 601274 [details] [diff] [review] (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit [Checked in: Comments 11 and 15] [Triage Comment] Test only change - approved for Aurora 12 and Beta 11. Note that beta 5 will be the last FF11 beta for which we'll accept test changes.
Attachment #601274 -
Flags: approval-mozilla-beta?
Attachment #601274 -
Flags: approval-mozilla-beta+
Attachment #601274 -
Flags: approval-mozilla-aurora?
Attachment #601274 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 14•12 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330540575.1330541423.13527.gz&fulltext=1 Linux comm-central-trunk debug test mochitests-2/5 on 2012/02/29 10:36:15 { 14453 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 11118ms } V.Fixed
Status: RESOLVED → VERIFIED
Keywords: checkin-needed
Whiteboard: [perma-orange] → [c-n: 30b4f99a137c to m-a and m-b] [perma-orange]
Comment 15•12 years ago
|
||
Comment on attachment 601274 [details] [diff] [review] (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit [Checked in: Comments 11 and 15] http://hg.mozilla.org/releases/mozilla-aurora/rev/b5ef80040f72 http://hg.mozilla.org/releases/mozilla-beta/rev/529498671c4c
Attachment #601274 -
Attachment description: (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comment 11] → (Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [c-n: 30b4f99a137c to m-a and m-b] [perma-orange] → [perma-orange]
Assignee | ||
Comment 16•12 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1330595464.1330597769.12144.gz&fulltext=1 Linux comm-aurora debug test mochitests-2/5 on 2012/03/01 01:51:04 { 14303 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 12910ms } seamonkey2.9: verified.
Assignee | ||
Comment 17•12 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1330650199.1330652139.30264.gz WINNT 5.2 comm-beta debug test mochitests-2/5 on 2012/03/01 17:03:19 firefox11: verified.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•