Last Comment Bug 718310 - Port |Bug 526998 - Implement F2 keyboard shortcut for renaming focused attachments when composing (on Windows and Unix)|
: Port |Bug 526998 - Implement F2 keyboard shortcut for renaming focused attach...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Composition (show other bugs)
: Trunk
: x86_64 Windows 7
: -- enhancement (vote)
: seamonkey2.9
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-15 10:09 PST by Jens Hatlak (:InvisibleSmiley)
Modified: 2012-01-22 14:26 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch [Checkin: Comment 5] (9.52 KB, patch)
2012-01-15 10:09 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
neil: superreview+
Details | Diff | Review

Description Jens Hatlak (:InvisibleSmiley) 2012-01-15 10:09:52 PST
Created attachment 588751 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2012-01-15 12:44:29 PST
(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 2012-01-15 19:46:19 PST
> 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) 2012-01-16 00:15:21 PST
(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 2012-01-22 13:53:29 PST
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) 2012-01-22 14:26:21 PST
Comment on attachment 588751 [details] [diff] [review]
patch [Checkin: Comment 5]

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

Note You need to log in before you can comment on or make changes to this bug.