Closed Bug 711085 Opened 8 years ago Closed 8 years ago

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

Categories

(Thunderbird :: Testing Infrastructure, defect)

x86
All
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 12.0

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(1 file, 1 obsolete file)

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: nobody → mconley
Target Milestone: --- → Thunderbird 11.0
Attached patch Patch v1 (obsolete) — Splinter Review
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)
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 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+
(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.
Thanks for the review, squib!  I've corrected that comment.
Attachment #584984 - Attachment is obsolete: true
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/7b724a79de0c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 11.0 → Thunderbird 12.0
You need to log in before you can comment on or make changes to this bug.