TEST-UNEXPECTED-FAIL | /builds/slave/test/build/mozmill/message-header/test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_reorder_buttons

RESOLVED FIXED in Thunderbird 33.0

Status

Thunderbird
Mail Window Front End
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jcranmer, Assigned: Gijs)

Tracking

({intermittent-failure, regression})

unspecified
Thunderbird 33.0
x86_64
Linux
intermittent-failure, regression

Thunderbird Tracking Flags

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

Details

Attachments

(4 attachments)

(Reporter)

Description

4 years ago
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

4 years ago
Blocks: 1000514
See Also: → bug 1000514
(Reporter)

Updated

4 years ago
See Also: bug 1000514
(Reporter)

Comment 1

4 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

4 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

4 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

4 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?
(Assignee)

Comment 6

4 years ago
Created attachment 8449829 [details] [diff] [review]
add pref for customizeToolbars security check,
Attachment #8449829 - Flags: review?(dao)
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

4 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

4 years ago
Created attachment 8450185 [details] [diff] [review]
adjust pref for test,

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

4 years ago
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+
(Assignee)

Comment 12

4 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

4 years ago
This is still broken. I don't know why. :-\
(Assignee)

Comment 14

4 years ago
Created attachment 8450653 [details] [diff] [review]
include pref in more tests,

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+
(Assignee)

Comment 17

4 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.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
(Assignee)

Comment 18

4 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

4 years ago
Created attachment 8450951 [details] [diff] [review]
Folded patch for comm-aurora/beta
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)
(Assignee)

Comment 22

4 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 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 25

4 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)
https://hg.mozilla.org/releases/comm-aurora/rev/1041f0fd1498
https://hg.mozilla.org/releases/comm-beta/rev/345e72f40b3d
status-thunderbird31: --- → fixed
status-thunderbird32: --- → fixed
tracking-thunderbird31: ? → +
tracking-thunderbird32: ? → +
Flags: needinfo?(standard8)
status-thunderbird33: --- → fixed
Target Milestone: --- → Thunderbird 33.0
You need to log in before you can comment on or make changes to this bug.