Closed Bug 1033568 Opened 5 years ago Closed 5 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, critical)

x86_64
Linux
defect
Not set
critical

Tracking

(thunderbird31+ fixed, thunderbird32+ fixed, thunderbird33 fixed)

RESOLVED FIXED
Thunderbird 33.0
Tracking Status
thunderbird31 + fixed
thunderbird32 + fixed
thunderbird33 --- fixed

People

(Reporter: jcranmer, Assigned: Gijs)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(4 files)

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 (?)
See Also: → CVE-2014-1561
See Also: CVE-2014-1561
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.
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
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.
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?
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+
(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
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)
Keywords: regression
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+
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.
This is still broken. I don't know why. :-\
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 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+
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.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
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?
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+
Gijs, can you explain how this impacts Firefox? Thanks
Flags: needinfo?(gijskruitbosch+bugs)
(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 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+
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)
Target Milestone: --- → Thunderbird 33.0
You need to log in before you can comment on or make changes to this bug.