719196 - Port |Bug 712421 - allow pasting a URL in the download manager window to download it|

Status: RESOLVED FIXED

(SeaMonkey :: Download & File Handling, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Neil, this should sound familiar. ;-)

Comment 1

[[e-mail]]

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.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2

(In reply to [e-mail] 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.

Comment 3

[[e-mail]]

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

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2a

Comment 5

[[e-mail]]

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

[[e-mail]]

Comment on attachment 589997 [details] [diff] [review]
patch v2a

r=me with the above fixed.

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] 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.

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2b [Checkin: Comment 9]

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 590325 [details] [diff] [review]
patch v2b [Checkin: Comment 9]

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