718310 - Port |Bug 526998 - Implement F2 keyboard shortcut for renaming focused attachments when composing (on Windows and Unix)|

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch [Checkin: Comment 5]

SSIA. Differences to what they did for TB:

* TB uses preprocessing, I made use of our platformMailnewsOverlay to exclude Mac
* they rewrote the whole controllers, I sticked to what we always do
* I didn't add Rename Attachment to the menus or change any l10n
* I didn't remove the onkeypress line from the attachmentlist (should I?)

Comment 1

[[e-mail]]

(In reply to Jens Hatlak from comment #0)
> * TB uses preprocessing, I made use of our platformMailnewsOverlay to exclude Mac
Of course :-)

> * they rewrote the whole controllers, I sticked to what we always do
If we were to go with anything like their system, I would actually have a JS object that converts from the command-object-object to the controller object, rather than duplicating the supports/do/isCommandEnabled logic.

> * I didn't add Rename Attachment to the menus or change any l10n
> * I didn't remove the onkeypress line from the attachmentlist (should I?)
Doesn't seem relevant to the keyboard shortcut, so followup bugs, perhaps.
The controller refactoring isn't really relevant either, but it was needed anyway to fix pressing delete with the From field focused ;-)

Comment 2

[Philip Chee]

> Bug 718310 - Port |Bug 526998
Bug 526998 got backed out.
New patch in attachment 588761 [details] [diff] [review]
Interdiff in attachment 588763 [details] [diff] [review]

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Philip Chee from comment #2)
> > Bug 718310 - Port |Bug 526998
> Bug 526998 got backed out.

Yeah I know. We don't have test that was failing (and needed to be adapted) and I didn't introduce new strings, so all is well. :-)

Comment 4

[Karsten Düsterloh]

Comment on attachment 588751 [details] [diff] [review]
patch [Checkin: Comment 5]

>+var attachmentBucketController =

Since controllers are global objects, (new) controllers' names should have the g prefix, i.e. gAttachmentBucketController.

r=me with that.

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 588751 [details] [diff] [review]
patch [Checkin: Comment 5]

http://hg.mozilla.org/comm-central/rev/e51b6e8ee03b