[SeaMonkey] permanent "dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out."

VERIFIED FIXED in Firefox 11

Status

()

Core
DOM
P2
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla13
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox10 wontfix, firefox11 verified, firefox12 verified, firefox-esr10 affected)

Details

(Whiteboard: [perma-orange], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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

6 years ago
Created attachment 595119 [details] [diff] [review]
(Av1) Add missing 'return', Set needed preference, Improve some nits
Attachment #595119 - Flags: review?(mounir)
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

6 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

6 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

6 years ago
Attachment #595119 - Flags: review?(mounir)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 6

6 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

6 years ago
Created attachment 598997 [details] [diff] [review]
(Av2) Add missing 'return', Set needed preference, Improve some nits

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 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

6 years ago
Created attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

Av2, with comment 8 suggestion(s).
Attachment #598997 - Attachment is obsolete: true
Attachment #601274 - Flags: review?(mounir)
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

6 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

6 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

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
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

6 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 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]
status-firefox11: affected → fixed
status-firefox12: affected → fixed
Keywords: checkin-needed
Whiteboard: [c-n: 30b4f99a137c to m-a and m-b] [perma-orange] → [perma-orange]
(Assignee)

Comment 16

6 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.
status-firefox12: fixed → verified
(Assignee)

Comment 17

6 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.
status-firefox11: fixed → verified
You need to log in before you can comment on or make changes to this bug.