Attachment tests are never run as part of mozmilltests.list, and are broken

RESOLVED FIXED in Thunderbird 12.0

Status

Thunderbird
Testing Infrastructure
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

Trunk
Thunderbird 12.0
x86
All
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Funny story:  it looks like we've never run the attachment tests as part of mozmilltests.list.

That's not good.

Even worse, when I ran the attachment tests, a bunch of them in test-attachment-menus.js failed.

I've got a fix for this - I'll post it up soon.
(Assignee)

Updated

6 years ago
Assignee: nobody → mconley
(Assignee)

Updated

6 years ago
Target Milestone: --- → Thunderbird 11.0
(Assignee)

Comment 1

6 years ago
Created attachment 584984 [details] [diff] [review]
Patch v1

Here's my first run at a fix.  Mozmill tests being run on try here:  http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=448128102300
Attachment #584984 - Flags: review?(squibblyflabbetydoo)
(Assignee)

Comment 2

6 years ago
Also, note the hackery in order to get the context menu to appear correctly in test-attachment-menus.js... are we sure we only want the icon / filename to have the context menu when right clicking?  Because we currently seem to ignore right-clicking on the file size.

Comment 3

6 years ago
Comment on attachment 584984 [details] [diff] [review]
Patch v1

Review of attachment 584984 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch on this! Assuming the tests pass now, r=me with the following nit about the comment.

::: mail/test/mozmill/attachment/test-attachment-menus.js
@@ +329,5 @@
> +
> +  // So this next bit is a little ugly.  The attachment names are currently
> +  // the only items that, when right clicking, bring up the context menus...
> +  // so, in order to open these up, we have to drag out the attachment name
> +  // by querying for it via getAnonymousElementByAttribute.

This part isn't quite true: what's really happening is that the <attachmentitem> ignores pointer events on Windows and Linux, but the icon and nameselection (a wrapper around the name) don't ignore them. (This is to allow click-through to work when clicking on whitespace).

I think it'd be more accurate to say "Attachment items themselves ignore pointer events, but the name always responds to them, so get the element for the name first."
Attachment #584984 - Flags: review?(squibblyflabbetydoo) → review+

Comment 4

6 years ago
(In reply to Mike Conley (:mconley) from comment #2)
> Also, note the hackery in order to get the context menu to appear correctly
> in test-attachment-menus.js... are we sure we only want the icon / filename
> to have the context menu when right clicking?  Because we currently seem to
> ignore right-clicking on the file size.

We could add CSS to make the file size be clickable too. I don't have any strong opinions there. We might also want to make some other changes, e.g. giving the attachment list Aero theming.
(Assignee)

Comment 5

6 years ago
Created attachment 585762 [details] [diff] [review]
Patch v2 (r+'d by squib)

Thanks for the review, squib!  I've corrected that comment.
Attachment #584984 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/7b724a79de0c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Target Milestone: Thunderbird 11.0 → Thunderbird 12.0
(Assignee)

Updated

6 years ago
Depends on: 715488
(Assignee)

Updated

6 years ago
Blocks: 715490
You need to log in before you can comment on or make changes to this bug.