876268 - Permanent orange: TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\mozmill\tabmail\test-tabmail-customize.js | test-tabmail-customize.js::teardownModule

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Jens Müller (:tessarakt)]

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/mozmill/tabmail/test-tabmail-customize.js | test-tabmail-customize.js::teardownModule

SUMMARY-UNEXPECTED-FAIL | test-tabmail-customize.js | test-tabmail-customize.js::teardownModule
  EXCEPTION: window is not defined
    at: EventUtils.js line 228
       synthesizeMouse EventUtils.js 228
       MozMillController.prototype.mouseEvent controller.js 515
       MozMillController.prototype.click controller.js 531
       wrapperFunc test-window-helpers.js 1017
       CustomizeDialogHelper_restoreDefaultButtons test-customization-helpers.js 101
       teardownModule test-tabmail-customize.js 32
       teardownModule test-folder-display-helpers.js 326
       Runner.prototype.wrapper frame.js 577
       Runner.prototype._runTestModule frame.js 674
       Runner.prototype.runTestModule frame.js 698
       Runner.prototype.runTestDirectory frame.js 522
       runTestDirectory frame.js 704
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183

Comment 1

[Jens Müller (:tessarakt)]

http://mxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension/resource/stdlib/EventUtils.js#225

When !aWindow holds, synthesizeMouse tries to take the global variable window instead. But this is not defined. Is there any way this can work? Who is supposed to put it in EventUtils.js's scope? Do script loaded with Cu.import have access to a caller's scope?!

The call is from a function in controller.js:

http://mxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension/resource/modules/controller.js#514

Apparently element.ownerDocument.defaultView is null in those cases.

So we have two different problems here:
1. synthesizeMouse relying on a window property of the global object.
2. MozMillController.prototype.mouseEvent passing a null value.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=876089#c23

Comment 2

[Jens Müller (:tessarakt)]

(In reply to Jens Müller from comment #1)
> So we have two different problems here:
> 1. synthesizeMouse relying on a window property of the global object.
> 2. MozMillController.prototype.mouseEvent passing a null value.

And of course:
3. Why is element.ownerDocument.defaultView null at that point? Is that something we can safely ignore?

Comment 3

[Jens Müller (:tessarakt)]

Interesting: http://mxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/patches/eventUtils.patch

Apparently, this was turned into a module (or is turned into a module by this patch), and at that point someone noticed that the window has to be passed in.

Btw, this patch is used here: http://mxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/scripts/sync_dependencies.py#49 . I don't really understand what it is used for ...

Comment 4

[Jens Müller (:tessarakt)]

(In reply to Jens Müller from comment #3)
> I don't really understand what it is used
> for ...

I do now - semi-automatic update from the latest upstream version.

Comment 5

[:aceman]

The test fails permanently on Linux and Windows (but not OS X) since May 22, commit 72bf76257a34. But it is more probably that something changed in mozilla-central at that time (between Wed May 22 13:22:06 2013 PDT and Wed May 22 06:55:09 2013 PDT) as seen at https://tbpl.mozilla.org/?tree=Thunderbird-Trunk.

Comment 6

[Mark Banner (:standard8)]

The actual failure is a few lines above the window one:

Test Failure: a != b: 'wrapper-menubar-items,wrapper-spring13693112290111' != 'menubar-items,spring'.
TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\mozmill\tabmail\test-tabmail-customize.js | test-tabmail-customize.js::test_redirects_toolbarbutton_drops
TEST-START | c:\talos-slave\test\build\mozmill\tabmail\test-tabmail-customize.js | teardownModule
Step Pass: {"function": "controller.click()"}
Test Failure: window is not defined

Using a link like this also helps:

http://clicky.visophyte.org/tools/arbpl-mozmill-standalone/?log=http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2013/05/2013-05-23-03-09-40-comm-central/comm-central_win7_test-mozmill-bm72-tests1-windows-build20.txt.gz

(this test is right at the bottom though!)

A slightly narrower regression range, is that it was something in mozilla-central, is most likely in this range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6066b9d92032&tochange=00b264c7cced

(obtained via looking at the nightly build results, and a bit of luck with the Win64 builder still having info there).

Comment 7

[:aceman]

Not sure if Firefox Australis changes affect TB in any way (http://www.gijsk.com/blog/2013/05/you-dont-know-what-youve-got-till-its-gone-or-tab-load-detection-gone-wrong/ ) ?

Comment 8

[Mark Banner (:standard8)]

Maybe Mike can help us here. The line that is failing now is:

http://hg.mozilla.org/comm-central/annotate/b63bf044ddcc/mail/test/mozmill/shared-modules/test-customization-helpers.js#l107

and it looks like there's be a "wrapper-" prefix to all the elements, which this code has:

http://hg.mozilla.org/comm-central/annotate/b63bf044ddcc/mail/test/mozmill/tabmail/test-tabmail-customize.js#l95

but its strange we seem to require this elsewhere now?

Comment 9

[:aceman]

A "wrapper-" prefix is added to toolbar elements when they are in the customize dialog. But I haven't yet checked if that is what happens here.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Correct - when we enter customize mode, all toolbar items and buttons are wrapped in toolbarpaletteitem nodes and those nodes are given the ID wrapper-[original item ID].

So if you're still seeing those wrapper nodes, it means that we've not yet properly exited customization mode by the time we do the element ID comparison.

Comment 11

[:aceman]

Attachment: patch

This fixes it for me. Must be applied on top of patch in bug 871266 (so that the customize dialog has a windowType).

Tessarakt, can you confirm the fix?

Comment 12

[:aceman]

Attachment: patch v2

OK, this new version also fixes all the other customize failures seen on https://tbpl.mozilla.org/?tree=Thunderbird-Trunk :
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_check_default
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_reorder_buttons
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_separate_window
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_remove_buttons
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_dialog_style
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_change_button_style

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 757181 [details] [diff] [review]
patch v2

Review of attachment 757181 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me, and try server looks better as well.

Comment 14

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/a2966564a3c2

Comment 15

[Mark Banner (:standard8)]

I think this may have actually created a new orange on Mac:

TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-widget/test-message-filters.js | test_customize_toolbar_doesnt_double_get_mail_menu
Step Pass: {"function": "controller.click()"}
Test Failure: Timeout waiting for popup to open
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-widget/test-message-filters.js | test-message-filters.js::test_customize_toolbar_doesnt_double_get_mail_menu


SUMMARY-UNEXPECTED-FAIL | test-message-filters.js | test-message-filters.js::test_customize_toolbar_doesnt_double_get_mail_menu
  EXCEPTION: Timeout waiting for popup to open
    at: utils.js line 447
       TimeoutError utils.js 447
       waitFor utils.js 485
       wait_for_popup_to_open test-folder-display-helpers.js 1361
       check_getAllNewMsgMenu test-message-filters.js 107
       test_customize_toolbar_doesnt_double_get_mail_menu test-message-filters.js 120
       Runner.prototype.wrapper frame.js 582
       Runner.prototype._runTestModule frame.js 652
       Runner.prototype.runTestModule frame.js 698
       Runner.prototype.runTestDirectory frame.js 522
       runTestDirectory frame.js 704
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183

Comment 16

[Mark Banner (:standard8)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=23959203&tree=Thunderbird-Trunk#error7

Comment 17

[:aceman]

I think it was caused by the patch in bug 871266. Please wait a bit with the backout.