Last Comment Bug 876268 - Permanent orange: TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\mozmill\tabmail\test-tabmail-customize.js | test-tabmail-customize.js::teardownModule
: Permanent orange: TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\mozmill\ta...
Status: RESOLVED FIXED
: intermittent-failure
Product: Thunderbird
Classification: Client Software
Component: Toolbars and Tabs (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 24.0
Assigned To: :aceman
:
Mentors:
Depends on: 871266
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-26 10:14 PDT by Jens Müller (:tessarakt)
Modified: 2013-06-25 05:20 PDT (History)
5 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (5.34 KB, patch)
2013-06-02 14:00 PDT, :aceman
no flags Details | Diff | Review
patch v2 (6.50 KB, patch)
2013-06-02 14:24 PDT, :aceman
standard8: review+
Details | Diff | Review

Description Jens Müller (:tessarakt) 2013-05-26 10:14:21 PDT
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) 2013-05-26 10:20:05 PDT
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) 2013-05-26 10:21:47 PDT
(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) 2013-05-26 11:33:18 PDT
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) 2013-05-26 12:35:42 PDT
(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 2013-05-27 00:10:24 PDT
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) 2013-05-30 00:19:46 PDT
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 2013-05-30 01:17:49 PDT
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) 2013-05-30 05:59:11 PDT
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 2013-05-30 07:23:19 PDT
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) - (Needinfo me!) 2013-06-02 08:58:06 PDT
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 2013-06-02 14:00:34 PDT
Created attachment 757177 [details] [diff] [review]
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 2013-06-02 14:24:44 PDT
Created attachment 757181 [details] [diff] [review]
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) 2013-06-10 04:28:06 PDT
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) 2013-06-10 04:31:02 PDT
https://hg.mozilla.org/comm-central/rev/a2966564a3c2
Comment 15 Mark Banner (:standard8) 2013-06-10 11:35:07 PDT
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 17 :aceman 2013-06-10 11:49:45 PDT
I think it was caused by the patch in bug 871266. Please wait a bit with the backout.

Note You need to log in before you can comment on or make changes to this bug.