Closed
Bug 719196
Opened 12 years ago
Closed 12 years ago
Port |Bug 712421 - allow pasting a URL in the download manager window to download it|
Categories
(SeaMonkey :: Download & File Handling, enhancement)
SeaMonkey
Download & File Handling
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.9
People
(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)
Details
Attachments
(1 file, 3 obsolete files)
4.15 KB,
patch
|
InvisibleSmiley
:
review+
|
Details | Diff | Splinter Review |
Neil, this should sound familiar. ;-)
Attachment #589613 -
Flags: review?(neil)
Comment 1•12 years ago
|
||
Unsurprisingly you failed to read bug 712421 comment 4 though. key_paste needs command="cmd_paste" (not sure why it doesn't get it automatically). This then needs to be handled in dlTreeController.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #1) > Unsurprisingly you failed to read bug 712421 comment 4 though. Wrong. Citing: "SeaMonkey's download manager uses a proper command controller which definitely won't conflict with the textbox" - skimming it, I read that as "there is no issue with this approach with SeaMonkey". Sure, with your explanation or looking deeper into the code it can be read differently, but it wasn't exactly crystal clear. > key_paste needs command="cmd_paste" (not sure why it doesn't get it > automatically). This then needs to be handled in dlTreeController. I wasn't sure how much of the pre-checks you'll require, so please give me some advice what you'd like to see for isCommandEnabled and doCommand respectively if you don't like the single function call approach.
Attachment #589613 -
Attachment is obsolete: true
Attachment #589613 -
Flags: review?(neil)
Attachment #589642 -
Flags: review?(neil)
Comment 3•12 years ago
|
||
Comment on attachment 589642 [details] [diff] [review] patch v2 > case "cmd_selectAll": > case "cmd_clearList": >+ case "cmd_paste": Nit: cmd_paste before cmd_selectAll, as it would be in editMenuCommands... >- "cmd_selectAll", "cmd_clearList"]; >+ "cmd_selectAll", "cmd_clearList", "cmd_paste"]; I think this change might be wrong... >+ <command id="cmd_paste" >+ oncommand="goDoCommand('cmd_paste');"/> because editMenuCommands already provides cmd_paste... >+ <key id="key_paste" Nit: keys in edit menu order too, so after key_copy please. [Hmm, I wonder why we don't use editMenuKeys...]
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #589642 -
Attachment is obsolete: true
Attachment #589642 -
Flags: review?(neil)
Attachment #589997 -
Flags: review?(neil)
Comment 5•12 years ago
|
||
Comment on attachment 589997 [details] [diff] [review] patch v2a D'oh, I need to update my tree for bug 712421 first, of course... >+ case "cmd_paste": >+ handlePaste(); Bah, I forgot to ask for a break; last time... > <key id="key_cut"/> > <key id="key_copy"/> >+ <key id="key_paste" >+ command="cmd_paste" >+ key="V" >+ modifiers="accel"/> > <key id="key_play" > key=" " > command="cmd_play"/> > <key id="key_delete"/> > <key id="key_delete2"/> > <key id="key_selectAll"/> The lack of attributes suggests we might pick up some automatically - please check and remove any you don't need.
Comment 6•12 years ago
|
||
Comment on attachment 589997 [details] [diff] [review] patch v2a r=me with the above fixed.
Attachment #589997 -
Flags: review?(neil) → review+
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #5) > >+ case "cmd_paste": > >+ handlePaste(); > Bah, I forgot to ask for a break; last time... Well, I made the mistake in the first place. ;-) > >+ <key id="key_paste" > >+ command="cmd_paste" > >+ key="V" > >+ modifiers="accel"/> > The lack of attributes suggests we might pick up some automatically - please > check and remove any you don't need. Indeed we get "key" and "modifiers" from elsewhere, probably utilityOverlay.xul (and I guess the platformHTMLBindings.xml reference there doesn't apply to key_paste). Will remove these two.
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #589997 -
Attachment is obsolete: true
Attachment #590325 -
Flags: review+
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 590325 [details] [diff] [review] patch v2b [Checkin: Comment 9] http://hg.mozilla.org/comm-central/rev/d3222f251e1c
Attachment #590325 -
Attachment description: patch v2b → patch v2b [Checkin: Comment 9]
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.9
You need to log in
before you can comment on or make changes to this bug.
Description
•