Closed
Bug 1033568
Opened 10 years ago
Closed 10 years ago
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/mozmill/message-header/test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_reorder_buttons
Categories
(Thunderbird :: Mail Window Front End, defect)
Tracking
(thunderbird31+ fixed, thunderbird32+ fixed, thunderbird33 fixed)
RESOLVED
FIXED
Thunderbird 33.0
People
(Reporter: jcranmer, Assigned: Gijs)
References
Details
(Keywords: intermittent-failure, regression)
Attachments
(4 files)
4.02 KB,
patch
|
dao
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1.99 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
3.22 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
5.80 KB,
patch
|
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
Effectively, dragging and dropping the widgets to customize the toolbar is broken. The regression was confirmed via a rather annoying hg bisect. [Filed in such a way so that I can star the several broken mozmill tests] Regressed by bug 1000514, which I can't mark as blocking (?)
Reporter | ||
Updated•10 years ago
|
Blocks: CVE-2014-1561
See Also: → CVE-2014-1561
Reporter | ||
Updated•10 years ago
|
See Also: CVE-2014-1561 →
Reporter | ||
Comment 1•10 years ago
|
||
Correction: my inability to customize was related to something in the profile; a fresh profile could customize the window. So it's really just a mozmill synthesized dnd issue.
Assignee | ||
Comment 2•10 years ago
|
||
I can look at this, although if mconley already knows what's up then that might be quicker. I expect we can probably just fake the property from the test? But maybe I'm too optimistic.
Assignee: nobody → gijskruitbosch+bugs
Reporter | ||
Comment 3•10 years ago
|
||
The patch in the other bug is on track to port to m-a and m-b, so I think we'll need to get the eventual patch here tracked as well.
tracking-thunderbird31:
--- → ?
tracking-thunderbird32:
--- → ?
Assignee | ||
Comment 4•10 years ago
|
||
I can't get mozmill tests to run locally (they open Thunderbird and then just sit there, with a bunch of errors in the console), but looking at the code, I suspect this will either need a similar pref solution as the Firefox tests did, or we need to seriously rearch the mozmill dnd tests... Can someone point me to an example thunderbird mozmill test that toggles a pref for the purposes of the test?
Reporter | ||
Comment 5•10 years ago
|
||
<http://dxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/test-message-header.js#240>
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8449829 -
Flags: review?(dao)
Comment 7•10 years ago
|
||
Comment on attachment 8449829 [details] [diff] [review] add pref for customizeToolbars security check, Since this is testing-only and we don't expect QA engineers or anyone else to manually flip the pref, I don't think this should be added to all.js. Instead, just wrap the three lines added to isUnwantedDragEvent in try/catch, and add a blank line after that while you're at it.
Attachment #8449829 -
Flags: review?(dao) → review+
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #7) > Comment on attachment 8449829 [details] [diff] [review] > add pref for customizeToolbars security check, > > Since this is testing-only and we don't expect QA engineers or anyone else > to manually flip the pref, I don't think this should be added to all.js. > Instead, just wrap the three lines added to isUnwantedDragEvent in > try/catch, and add a blank line after that while you're at it. Done. remote: https://hg.mozilla.org/integration/fx-team/rev/4f935bdb29fe
Status: NEW → ASSIGNED
Keywords: leave-open
Assignee | ||
Comment 9•10 years ago
|
||
This should then do. Can you test this, seeing as I can't get mozmill tests to run at all?
Attachment #8450185 -
Flags: review?(Pidgeot18)
Updated•10 years ago
|
Keywords: regression
Comment 11•10 years ago
|
||
Comment on attachment 8450185 [details] [diff] [review] adjust pref for test, Review of attachment 8450185 [details] [diff] [review]: ----------------------------------------------------------------- Looks good - I think we'll want to add something similar to this test too: https://mxr.mozilla.org/comm-central/source/mail/test/mozmill/tabmail/test-tabmail-customize.js#56 Thanks!
Attachment #8450185 -
Flags: review?(Pidgeot18) → review+
Assignee | ||
Comment 12•10 years ago
|
||
remote: https://hg.mozilla.org/comm-central/rev/d2c28df60dab This should be fixed. Let's hang on until we have green (or at least, these failures are gone) on tbpl.
Assignee | ||
Comment 13•10 years ago
|
||
This is still broken. I don't know why. :-\
Assignee | ||
Comment 14•10 years ago
|
||
Oh, wait, yes I do. There are more of these. Ugh. Sorry for not noticing this first time 'round. Judging by http://mxr.mozilla.org/comm-central/search?string=drag_n_drop&find=mozmill this should be everything.
Attachment #8450653 -
Flags: review?(mconley)
Comment 15•10 years ago
|
||
Comment on attachment 8450653 [details] [diff] [review] include pref in more tests, Review of attachment 8450653 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Gijs!
Attachment #8450653 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 16•10 years ago
|
||
remote: https://hg.mozilla.org/comm-central/rev/0ae1f056a7a5
Assignee | ||
Comment 17•10 years ago
|
||
3/4 linux builds and all windows XP builds are green, so I think it's safe to say this is now fixed. I'll ask for uplift next.
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8449829 [details] [diff] [review] add pref for customizeToolbars security check, Approval Request Comment [Feature/regressing bug #]: bug 1000514 [User impact if declined]: broken thunderbird tests [Describe test coverage new/current, TBPL]: those thunderbird tests [Risks and why]: none [String/UUID change made/needed]: none
Attachment #8449829 -
Flags: approval-mozilla-beta?
Attachment #8449829 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 19•10 years ago
|
||
Comment 20•10 years ago
|
||
Comment on attachment 8450951 [details] [diff] [review] Folded patch for comm-aurora/beta a=Standard8 for landing these on comm-aurora/beta (I can't approve the mozilla ones)
Attachment #8450951 -
Flags: approval-comm-beta+
Attachment #8450951 -
Flags: approval-comm-aurora+
Comment 21•10 years ago
|
||
Gijs, can you explain how this impacts Firefox? Thanks
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #21) > Gijs, can you explain how this impacts Firefox? Thanks It does not (except potentially for add-ons), but it impacts other toolkit consumers, and we've uplifted the original bug, which also doesn't impact Firefox (except potentially for add-ons). So we should uplift this as well. All it does is make some code that we added listen for a preference, which we can set in tests so that the tests of the respective toolkit consumers don't break.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 23•10 years ago
|
||
Comment on attachment 8449829 [details] [diff] [review] add pref for customizeToolbars security check, OK. Taking because 31 is an ESR release.
Attachment #8449829 -
Flags: approval-mozilla-beta?
Attachment #8449829 -
Flags: approval-mozilla-beta+
Attachment #8449829 -
Flags: approval-mozilla-aurora?
Attachment #8449829 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 24•10 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/62c43665c993 remote: https://hg.mozilla.org/releases/mozilla-beta/rev/73ce1f980083
Assignee | ||
Comment 25•10 years ago
|
||
Mark said he could uplift to comm-a/b per IRC discussion; needinfo'ing so this doesn't fall through the cracks :-)
Flags: needinfo?(standard8)
Comment 26•10 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/1041f0fd1498 https://hg.mozilla.org/releases/comm-beta/rev/345e72f40b3d
Updated•10 years ago
|
status-thunderbird33:
--- → fixed
Target Milestone: --- → Thunderbird 33.0
You need to log in
before you can comment on or make changes to this bug.
Description
•