Closed Bug 1332708 Opened 8 years ago Closed 8 years ago

Mozmill failure on 20-01-2017: Timeouts and utils.waitFor() failing

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: jorgk-bmo, Assigned: enndeakin)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Sadly Mozmill is currently red due to bug 1329288, so additional bustage is hard to see. Last good: Thu Jan 19, 2016, 18:53:22 First bad: Fri Jan 20, 2016, 8:08:48 https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=2d4babd80f8669f3bea94c87edf4dbf6da5d0d12 TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testAnnualRecurrence.js | testAnnualRecurrence.js::testAnnualRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testBiweeklyRecurrence.js | testBiweeklyRecurrence.js::testBiweeklyRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testDailyRecurrence.js | testDailyRecurrence.js::testDailyRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testLastDayOfMonthRecurrence.js | testLastDayOfMonthRecurrence.js::testLastDayOfMonthRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testWeeklyNRecurrence.js | testWeeklyNRecurrence.js::testWeeklyNRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testWeeklyUntilRecurrence.js | testWeeklyUntilRecurrence.js::testWeeklyUntilRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cal-recurrence\testWeeklyWithExceptionRecurrence.js | testWeeklyWithExceptionRecurrence.js::testWeeklyWithExceptionRecurrence TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\testLocalICS.js | testLocalICS.js::testLocalICS What's going on here, MMD, can you please take a look.
Flags: needinfo?(makemyday)
There is also a failure in test-header-toolbar.js. Log says: INFO - SUMMARY-UNEXPECTED-FAIL | test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_change_button_style INFO - EXCEPTION: Timeout waiting for window to close! Some of the calendar failures say: EXCEPTION: waitFor: Timeout exceeded for '() => mozmill.utils.getWindows("Calendar:EventDialog").length == 0' So it looks like M-C changed some window handling. Regression range M-C Good: a3978751f45108ff1ae002ecebdc0fa23f Bad: aa3e49299a3aa5cb0db570532e3df9e75d https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a3978751f45108ff1ae002ecebdc0fa23f&tochange=aa3e49299a3aa5cb0db570532e3df9e75d
mozmake SOLO_TEST=message-header/test-header-toolbar.js mozmill-one fails locally with: SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\mail\test\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_change_button_style EXCEPTION: Timeout waiting for window to close! at: utils.js line 447 TimeoutError utils.js:447 13 waitFor utils.js:485 11 WindowWatcher_waitForWindowClose test-window-helpers.js:398 5 wait_for_window_close test-window-helpers.js:639 3 CustomizeDialogHelper_close test-customization-helpers.js:83 7 test_customize_header_toolbar_change_button_style test-header-toolbar.js:408 3 Runner.prototype.wrapper frame.js:585 9 Runner.prototype._runTestModule frame.js:655 9 Runner.prototype.runTestModule frame.js:701 3 Runner.prototype.runTestFile frame.js:534 3 runTestFile frame.js:713 3 Bridge.prototype._execFunction server.js:179 10 Bridge.prototype.execFunction server.js:183 16 Session.prototype.receive server.js:283 3 AsyncRead.prototype.onDataAvailable server.js:88 3 Aceman, you have a lot of experience with this. Could you check the regression range and give and/or give me a tip?
Flags: needinfo?(acelists)
Hmm, test-header-toolbar.js:408 is |gCDHelper.close(ctc);|. Let's see the calendar failures: Four have this failure: test-calendar-utils.js:328 which is controller.waitFor(() => mozmill.utils.getWindows("Calendar:EventDialog").length == 0); Twelve failures have this MozMillController.prototype.waitFor controller.js:687 which is utils.waitFor(callback, message, timeout, interval, thisObject); And one has WindowWatcher_waitForWindowClose test-window-helpers.js:398 which is utils.waitFor(() => this.monitorizeClose() So something is is wrong with waiting, not sure if it's only waiting for windows to close.
There have been a few of that timeout issues before (see comment in bug 980515) already, but that haven't been as massive as now. Markus, can you take a look at the calendar failures?
Flags: needinfo?(makemyday) → needinfo?(Mozilla)
I see that on Linux (32bit) the failure in TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cal-recurrence/testWeeklyWithExceptionRecurrence.js | testWeeklyWithExceptionRecurrence.js::testWeeklyWithExceptionRecurrence TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cal-recurrence/testWeeklyUntilRecurrence.js | testWeeklyUntilRecurrence.js::testWeeklyUntilRecurrence actually started since bug 980515 on Jan 7. So just for the experiment, have you tried pushing revert of bug 1332380 if that helps? Or the m-c version of it, bug 1312216.
Flags: needinfo?(acelists)
(In reply to :aceman from comment #5) > > So just for the experiment, have you tried pushing revert of bug 1332380 if > that helps? > Or the m-c version of it, bug 1312216. This are changes in the in-content prefs only. So I think this will have no influence here.
M-C rev 5a03c87a1725 couldn't be backed out easily, so I backed out https://hg.mozilla.org/mozilla-central/rev/30e79c2a8338 https://hg.mozilla.org/mozilla-central/rev/8a0dc12ec71f and https://hg.mozilla.org/mozilla-central/rev/5a03c87a1725, see: https://hg.mozilla.org/mozilla-central/filelog/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/modules/SelectParentHelper.jsm That partially backs out two bugs: bug 1321472 and bug 1311279 which is most likely not a great idea since these two bugs have six changesets in total. Anyway, with these changes backed out this mozmake SOLO_TEST=message-header/test-header-toolbar.js mozmill-one passes again. Neil and Mike, can you please lend a hand here. What do we need to do to get our tests going again?
Blocks: 1321472, 1311279
Flags: needinfo?(mconley)
Flags: needinfo?(enndeakin)
The failures all seem to happen within the event-Dialog. < controller.waitFor(() => mozmill.utils.getWindows("Calendar:EventDialog").length == 0); This is the part, where we wait for the Dialog to close. I'll try to reproduce it locally to see what happens.
Flags: needinfo?(Mozilla)
Markus: I wouldn't spend to much time on it. Something has changed in M-C, see my previous comment #8, and that either needs to be fixed on M-C or in the C-C Mozmill framework.
Keywords: regression
Summary: Massive Mozmill failure in Calendar on 20-01-2017 → Mozmill failure on 20-01-2017: Timeouts and utils.waitFor() failing
What happens if you just delete the parts I added to popup.xml in https://hg.mozilla.org/mozilla-central/rev/30e79c2a8338 and leave everything else as is? What happens if you leave every patch as is but just comment out the line that calls 'setCaptureAlways()'?
Flags: needinfo?(enndeakin)
(In reply to Neil Deakin from comment #11) > What happens if you just delete the parts I added to popup.xml in > https://hg.mozilla.org/mozilla-central/rev/30e79c2a8338 and leave everything > else as is? Removing the code in the <implementation> and <handlers> tags at the end of the file and therefore reducing that section to <binding id="popup-scrollbars" extends="chrome://global/content/bindings/popup.xml#popup"> <content> <xul:scrollbox class="popup-internal-box" flex="1" orient="vertical" style="overflow: auto;"> <children/> </xul:scrollbox> </content> </binding> makes the TB Mozmill test pass. I I haven't tried the Calendar ones. > What happens if you leave every patch as is but just comment out the line > that calls 'setCaptureAlways()'? Putting the removed section back (hg revert --all) and just taking out |this.setCaptureAlways();|, thus changing the section to: <![CDATA[ if (!this._draggingState) { this._draggingState = overItem ? this.DRAG_OVER_POPUP : this.DRAG_OVER_BUTTON; } ]]> also makes my test pass. I will ship off a try run of C-C with this one line removed from M-C (I can do that ;-)). Thanks for your help!
Flags: needinfo?(mconley)
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=309695d08c95c81f613e51f7416f82242e512dc1 Fingers crossed. Now, if that fixes the other Mozmill failures as well, what is the way forward?
Flags: needinfo?(enndeakin)
No longer blocks: 1311279
Removing |this.setCaptureAlways();| fixed all the Mozmill failures reported in this bug. The "Z" is still red due to a failure from bug 1329288 (which will be fixed soon). So what's the way forward?
Attached patch dragonlywhenopen (deleted) — Splinter Review
The issue is that these tests are trying to click on a menuitem while the menulist isn't open and the mouse capture is getting confused. This patch doesn't enable mouse capture when the popup isn't open. The test, of course, should really be waiting for the menulist's popup to open first if it is trying to click on items in it.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
You mean the TB tests? We can fix those if needed.
It seems to me in test-header-toolbar.js::test_customize_header_toolbar_change_button_style() we need to click(<menulist id="modelist">) before clicking the iconMode or textMode buttons.
For the calendar tests, at https://dxr.mozilla.org/comm-central/rev/9cb95ca4c3f7cbd473e78addb8e3607af6af8386/calendar/test/mozmill/shared-modules/test-calendar-utils.js# we should wait for the menulist to open, like: aController.waitFor(() => menulist.menupopup.state == "open", "menupoup didn't open")
(In reply to :aceman from comment #17) Please suggest the exact line you want me to add and it's location. (In reply to :aceman from comment #18) Where exactly would you like me to insert this line? I'm in favour of fixing the test *now* before the M-C change goes ahead, preferably today before the branch day tomorrow.
Flags: needinfo?(acelists)
(In reply to Jorg K (GMT+1) from comment #19) > (In reply to :aceman from comment #17) > Please suggest the exact line you want me to add and it's location. https://dxr.mozilla.org/comm-central/rev/9cb95ca4c3f7cbd473e78addb8e3607af6af8386/mail/test/mozmill/message-header/test-header-toolbar.js#405 : ctc.click(new elib.eid("modelist")); ctc.waitFor(() => ctc.window.document.getElementById("modelist").menupopup.state == "open", "menupoup didn't open"); The same at line 416. > (In reply to :aceman from comment #18) > Where exactly would you like me to insert this line? https://dxr.mozilla.org/comm-central/rev/9cb95ca4c3f7cbd473e78addb8e3607af6af8386/calendar/test/mozmill/shared-modules/test-calendar-utils.js#861
Flags: needinfo?(acelists)
Attached patch 1332708-fix-mozmill.patch for C-C (obsolete) (deleted) — Splinter Review
Like this? I can't try it right now since my machine is blocked for another hour by yet another recompile after yet another bustage fix.
Attachment #8829201 - Flags: feedback?(acelists)
BTW, what's "modelist"?
Comment on attachment 8829201 [details] [diff] [review] 1332708-fix-mozmill.patch for C-C Review of attachment 8829201 [details] [diff] [review]: ----------------------------------------------------------------- Yes, I think that is the missing logic. Maybe the 'new' before elib.eid() is not needed, or it will need to be elib.Elem(ctc.window.document.getElementById("")) so you may need to play with the syntax locally.
Attachment #8829201 - Flags: feedback?(acelists) → feedback+
(In reply to Jorg K (GMT+1) from comment #22) > BTW, what's "modelist"? As said in comment 17, just the id of the menulist element we want to open first before operating its menuitems. https://dxr.mozilla.org/comm-central/rev/a3978751f45108ff1ae002ecebdc0fa23fc52b84/mozilla/toolkit/content/customizeToolbar.xul#45
OK, now the calendar tests all fail with: EXCEPTION: menupoup didn't open (11 times seen in the file). And test_customize_header_toolbar_change_button_style says: EXCEPTION: elib.eid is not a constructor So not a total success. Sadly my local build broken, so I have to fix that first.
(In reply to Jorg K (GMT+1) from comment #26) > OK, now the calendar tests all fail with: > EXCEPTION: menupoup didn't open (11 times seen in the file). Not sure what is the problem here, try + aController.waitFor(() => menulist.menupopup + .state == "open", "menupopup didn't open, state="+menulist.menupopup.state); > And test_customize_header_toolbar_change_button_style says: > EXCEPTION: elib.eid is not a constructor > Try without the 'new'.
I will, thanks, my local build is still broken, I'm doing the fourth full build today. Maybe it was not working due to all the XPCOM glue changes and the fact that I had config option "disable sandboxing" set. Grrr. .menupop is the right thing to do? I haven't see this elsewhere.
(In reply to Jorg K (GMT+1) from comment #28) > .menupop is the right thing to do? I haven't see this elsewhere. I like it :) There wasn't an error about it. If should fetch the menupopup child of the menulist. Other way is to find the menupopup element via ID (if it has one) so it seems the first method is more robust (if we already have the parent menulist found).
(In reply to :aceman from comment #27) > Try without the 'new'. OK, I can work again. Without the 'new' I get: EXCEPTION: elib.eid is not a function
Next I tried: let ctc = gCDHelper.open(mc); let popup = ctc.window.document.getElementById("modelist"); ctc.click(new elib.Elem(popup)); ctc.waitFor(() => popup.state == "open", "menupoup didn't open, state="+popup.state); let iconMode = ctc.window.document.getElementById("main-box"). querySelector("[value*='icons']"); ctc.click(new elib.Elem(iconMode)); gCDHelper.close(ctc); Now I get the same as in calendar: EXCEPTION: menupoup didn't open, state=undefined Watching the test, the popup does show, but no item is clicked on it. I also tried // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let popup = ctc.window.document.getElementById("modelistpopup"); ctc.click(new elib.Elem(popup)); ctc.waitFor(() => popup.state == "open", "menupoup didn't open, state="+popup.state); let iconMode = ctc.window.document.getElementById("main-box"). querySelector("[value*='icons']"); ctc.click(new elib.Elem(iconMode)); gCDHelper.close(ctc); and that gave: EXCEPTION: menupoup didn't open, state=closed
Next attempt: // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let list = ctc.window.document.getElementById("modelist"); let popup = ctc.window.document.getElementById("modelistpopup"); ctc.click(new elib.Elem(list)); ctc.waitFor(() => popup.state == "open", "menupoup didn't open, state="+popup.state); let iconMode = ctc.window.document.getElementById("main-box"). querySelector("[value*='icons']"); ctc.click(new elib.Elem(iconMode)); gCDHelper.close(ctc); That fails on the .close(ctc). I don't actually understand what we're trying to click on. https://dxr.mozilla.org/comm-central/rev/a3978751f45108ff1ae002ecebdc0fa23fc52b84/mozilla/toolkit/content/customizeToolbar.xul#45 Both modelist and modeicons have value icons, so which one does the query selector select?? And what's the value*?
Next attempt: // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let list = ctc.window.document.getElementById("modelist"); let popup = ctc.window.document.getElementById("modelistpopup"); let icons = ctc.window.document.getElementById("modeicons"); ctc.click(new elib.Elem(list)); ctc.waitFor(() => popup.state == "open", "menupoup didn't open, state="+popup.state); ctc.click(new elib.Elem(icons)); gCDHelper.close(ctc); The menu clearly shows, but the click on items doesn't happen, so the .close fails. What does the .open(() actually open? I just remove the .close() calls from the original code and the test runs though and the fails after restoreDefaultButtons().
Last attempt: // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let list = ctc.window.document.getElementById("modelist"); let popup = ctc.window.document.getElementById("modelistpopup"); ctc.click(new elib.Elem(list)); ctc.waitFor(() => popup.state == "open", "menupoup didn't open, state="+popup.state); let icons = ctc.window.document.getElementById("modeicons"); ctc.click(new elib.Elem(icons)); ctc.waitFor(() => popup.state == "closed", "menupoup didn't close, state="+popup.state); gCDHelper.close(ctc); Gives: EXCEPTION: menupoup didn't close, state=open So the menu opens, but as much as you click, the menu stays open. I think I should give up since I obviously don't understand what - .open() should do - .close(0 should do - why the click doesn't have any effect on a menu that's visibly open. - why the whole thing words by just removing the .close() but then fails later.
Ah sorry, .eid() is on the controller, so try ctc.click(ctc.eid("modelist")) Both of the attempts in comment 31 have bugs that is why they didn't work. But thanks for trying. The version in comment 32 seems workable (just verbose). It seems to me it wants to click the item with id="modeicons" it just finds it via having the right value attribute (I didn't check what the * does in the selector). What do you mean it fails on .close(ctc)? What is the error?
(In reply to :aceman from comment #35) > What do you mean it fails on .close(ctc)? What is the error? EXCEPTION: Timeout waiting for window to close! Sorry, I can't find any working permutation as much as I have tried. The popup opens and the selected "base" value is icons, but the selected popup value is "Icons and Text". An can even manually click in the popup, but the popup doesn't close. In the end it times out. So summary: We have no problem opening the popup, but it just never closes.
I assume the .open() and .close() are methods to open and close the customization palette (dialog). https://dxr.mozilla.org/comm-central/rev/9cb95ca4c3f7cbd473e78addb8e3607af6af8386/mail/test/mozmill/shared-modules/test-customization-helpers.js#47 Thus the test opens the dialog, clicks a menulist in it, then an item in the menu. Then it needs to close the customization dialog. This repeats then. If it fails to close the dialog (or you removed the call) it may backfire later in the test.
Yet another attempt: // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let iconMode = ctc.window.document.getElementById("main-box"). querySelector("[value*='icons']"); ctc.click(ctc.eid("modelist")); ctc.waitFor(() => ctc.window.document.getElementById("modelist") .menupopup.state == "open", "menupoup didn't open"); ctc.click(new elib.Elem(iconMode)); gCDHelper.close(ctc); Fails on the .close() with error EXCEPTION: Timeout waiting for window to close! I repeat: Clicking on the icons item, selects the base value, but never changes the value in the popup which remains "Icons and Text". The popup also doesn't close. I have the impression that something is broken there. We do all the right things. We click on the list, we check that the state is open, we supposedly click in an open popup, which is visually open, but it never closes. Look at all the variations in the previous comments. Nothing works, well, it's basically all the same, just addressing the different element differently.
Wires crossed with comment #37. So if .close() wants to close the whole dialogue, that thoroughly fails since the popup menu is still up. And it doesn't respond to clicks, not even manual ones.
Attached image Picture showing the hung menu (deleted) —
Neil, as you can see from the flood of comments, I've spent a long time with this and couldn't find a working solution. You said in comment #15: > The test, of course, should really be waiting for the menulist's > popup to open first if it is trying to click on items in it. So I modified the code to do this: // Change the button style to icon (only) mode let ctc = gCDHelper.open(mc); let iconMode = ctc.window.document.getElementById("main-box"). querySelector("[value*='icons']"); ctc.click(ctc.eid("modelist")); ctc.waitFor(() => ctc.window.document.getElementById("modelist") .menupopup.state == "open", "menupoup didn't open"); dump(ctc.window.document.getElementById("modelist").value+" <== before click\n"); ctc.click(new elib.Elem(iconMode)); dump(ctc.window.document.getElementById("modelist").value+" <== after click\n"); gCDHelper.close(ctc); <=== 413 The menu is defined here: https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/content/customizeToolbar.xul#45 yet, TB adds another time "Text beside Icons" here: https://dxr.mozilla.org/comm-central/rev/9cb95ca4c3f7cbd473e78addb8e3607af6af8386/mail/base/content/mailCore.js#77 What I see in the test is what you can see in the attached picture. The popup opens and the original value is highlighted "Icons beside Text". Then the click is performed, and the value changes to "icons" as the debug produced by the code above shows: textbesideicon <== before click icons <== after click The popup never closes and in the end the test fails with EXCEPTION: Timeout waiting for window to close! at test_customize_header_toolbar_change_button_style test-header-toolbar.js:413 which is the line I marked. As you can see in comment #34, I had a version like let popup = ctc.window.document.getElementById("modelistpopup"); ... let icons = ctc.window.document.getElementById("modeicons"); ctc.click(new elib.Elem(icons)); ctc.waitFor(() => popup.state == "closed", "menupoup didn't close, state="+popup.state); gCDHelper.close(ctc); and that gave: EXCEPTION: menupoup didn't close, state=open So while we're happy to fix our tests where they do thing they shouldn't be doing, like simulating clicking on closed menu items, I have the impression that despite our best efforts, there is a bug in the M-C code. At the end, I tried your patch with the our modified test and our original test. The modified version which visibly brings up the popup menu still fails, but the original test which clicks on an item in a non-visible popup succeeds. So did I just spent time in vain trying to improve the tests?
I'm pretty sure element.click() doesn't synthesize a mouse event. It just sends the mouse events to the dom element, and does no default handling. Did you try test patch I posted?
Oops, my comment was so long that you didn't read to the end ;-( Let me repeat the end of comment #40: === At the end, I tried your patch with the our modified test and our original test. The modified version which visibly brings up the popup menu still fails, but the original test which clicks on an item in a non-visible popup succeeds. So did I just spent time in vain trying to improve the tests? === My aim was to fix the tests so that they pass without your patch, and I failed. With your patch, the original tests pass. But didn't you tell us we should really open the popup first?
We would want my patch regardless. I don't know what the test is trying to test, so I can't really answer exactly. If it is trying to verify the contents and behaviour of the menu and/or could be affected by other UI it should use lower level mouse synthesis. If it is just trying to test the code that would run when the menuitem is selected runs, then the test would probably be ok as is.
Neil, Jorg was trying the other test, where we NOW do proper controller.click(menulist element), see comment 40. The strange thing is all other menulists in TB seem to work fine (tests pass), just the 2 menulists in the failed tests (calendar something and one in the customize dialog).
(In reply to Neil Deakin from comment #43) > If it is just trying to test the code that would run when the > menuitem is selected runs, then the test would probably be ok as is. Yes, that's the aim. Run something from a menu and check the result. (In reply to :aceman from comment #44) > Neil, Jorg was trying the other test, where we NOW do proper > controller.click(menulist element), see comment 40. element.click() (comment #41) or controller.click(menulist element), I'm confused now.
(In reply to Jorg K (GMT+1) from comment #45) > (In reply to :aceman from comment #44) > > Neil, Jorg was trying the other test, where we NOW do proper > > controller.click(menulist element), see comment 40. > element.click() (comment #41) or controller.click(menulist element), I'm > confused now. We played with controller.click(menulist). The failing calendar tests use menulist.click() which Neil thinks may not be as great. We can rewrite that too if needed.
Neil, could you please get this reviewed, landed and uplifted to M-A. Our C-C and C-A trees look quite red due to the Mozmill failures. Sorry for "spamming" your bug with comments related to improving our tests. We can do this in a different bug later. Now my focus is on getting the trees green again.
Flags: needinfo?(enndeakin)
Flags: needinfo?(enndeakin)
Attachment #8829185 - Flags: review?(mconley)
Attachment #8829185 - Flags: review?(mconley) → review+
Sorry to be a nuisance here, but can we please get this landed (and uplifted). Please just set "checkin-needed" if you don't want to push it yourself.
Flags: needinfo?(enndeakin)
Attachment #8829201 - Attachment is obsolete: true
Component: General → Layout: Form Controls
Product: Thunderbird → Core
Pushed by neil@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dc004d33b0f9 open start dragging on a menuitem when dropdown is open, r=mconley
Flags: needinfo?(enndeakin)
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8829185 [details] [diff] [review] dragonlywhenopen I'm requesting uplift here on behalf of Neil, who can provide further details. Approval Request Comment [Feature/Bug causing the regression]: Bug 1321472 landed on mozilla53 [User impact if declined]: Firefox: Neil please fill in details Thunderbird: Mozmill tests failing. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Firefox: Neil please fill in details. Thunderbird: Yes, with this fix Thunderbird's Mozmill tests pass now. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No small change, a missing bit from bug 1321472, covered by test. [Why is the change risky/not risky?]: see above. [String changes made/needed]: None. As I understand comment #43 ("We would want my patch regardless.") correctly, this is also needed for Firefox. Neil can provide further details.
Flags: needinfo?(enndeakin)
Attachment #8829185 - Flags: approval-mozilla-aurora?
Blocks: 1334783
Thanks Jorg for filing the mozmill bug for calendar. I'll file the other one.
Blocks: 1334843
This change fixes a correctness check, where one can start a pseudo-drag when the menu isn't open causing wierd UI behaviour. That said, it is unlikely to happen outside of tests, as is the case with this bug.
Flags: needinfo?(enndeakin)
Julien, could you please approve this. Our Mozmill tests look terrible on Aurora due to this bug: https://treeherder.mozilla.org/#/jobs?repo=comm-aurora
Flags: needinfo?(jcristau)
Comment on attachment 8829185 [details] [diff] [review] dragonlywhenopen Fixes a mozmill test, let's uplift to Aurora53
Attachment #8829185 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: