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

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, defect)

Description

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

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.

Comment 1

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

Attachment: 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

Comment 2

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

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

[Jim Porter (:squib)]

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."

Comment 4

[Jim Porter (:squib)]

(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.

Comment 5

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

Attachment: Patch v2 (r+'d by squib)

Thanks for the review, squib!  I've corrected that comment.

Comment 6

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

Committed to comm-central as http://hg.mozilla.org/comm-central/rev/7b724a79de0c