Edit->Copy doesn't work in the SuiteRunner Error Console.

RESOLVED FIXED

Status

SeaMonkey
UI Design
--
minor
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

11 years ago
Bug 334997 added a menubar to the toolkit Error console when running in SeaMonkey. However the Edit->Copy menu item isn't working because the utilityOverlay pulled in overrides the toolkit error console "cmd_copy" command.

Neil on #seamonkey said to port the XPFE console's command controller. Patch coming up.
(Assignee)

Comment 1

11 years ago
Created attachment 251363 [details] [diff] [review]
Patch 1.0

Adds an inline script to consoleOverlay.xul to implement a command controller for "cmd_copy"
Assignee: guifeatures → philip.chee
Status: NEW → ASSIGNED
Attachment #251363 - Flags: superreview?(neil)
Attachment #251363 - Flags: review?(neil)

Comment 2

11 years ago
Comment on attachment 251363 [details] [diff] [review]
Patch 1.0

>+      isCommandEnabled: function (aCommand)
>+      supportsCommand: function (aCommand) 
>+      doCommand: function (aCommand)
Nit: correct order is supportsCommand, then isCommandEnabled, then doCommand

>+      window.removeEventListener("load", initConsoleOverlay, false);
Might not have to do this in a load event.

There's still a problem in that the menuitem isn't enabling when the evaluator text box has focus, because the toolkit version of UpdateCopyMenu is different.
(Assignee)

Comment 3

11 years ago
Created attachment 251384 [details] [diff] [review]
Patch 1.1

> (From update of attachment 251363 [details] [diff] [review])
>>+      isCommandEnabled: function (aCommand)
>>+      supportsCommand: function (aCommand)
>>+      doCommand: function (aCommand)
> Nit: correct order is supportsCommand, then isCommandEnabled, then doCommand

That was the order in the XPFE console.js introduced in Bug 67647. You didn't mention anything about the order then.

> >+      window.removeEventListener("load", initConsoleOverlay, false);
> Might not have to do this in a load event.

Won't this leak when the window closes?

> There's still a problem in that the menuitem isn't enabling when the evaluator
> text box has focus, because the toolkit version of UpdateCopyMenu is different.

Is it just a matter of overriding the toolkit [u|U]pdateCopyMenu with the XPFE version? This seems to work and doesn't seem to affect the ability to copy the console items.
Attachment #251363 - Attachment is obsolete: true
Attachment #251363 - Flags: superreview?(neil)
Attachment #251363 - Flags: review?(neil)
(Assignee)

Comment 4

11 years ago
Comment on attachment 251384 [details] [diff] [review]
Patch 1.1

Index: consoleOverlay.xul
===================================================================
RCS file: /cvsroot/mozilla/suite/common/consoleOverlay.xul,v
retrieving revision 1.1
diff -u -8 -p -r1.1 consoleOverlay.xul
--- consoleOverlay.xul	25 Apr 2006 14:33:55 -0000	1.1
+++ consoleOverlay.xul	13 Jan 2007 17:57:10 -0000
@@ -46,16 +46,72 @@
 <!ENTITY % consoleOverlayDTD SYSTEM "chrome://communicator/locale/consoleOverlay.dtd">
 %consoleOverlayDTD;
 
 ]>
 
 <overlay id="consoleOverlay"
          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
+  <script type="application/x-javascript"><![CDATA[
+    /* :::::::: Command Controller for the Window ::::::::::::::: */
+
+    var ConsoleController =
+    {
+      supportsCommand: function (aCommand) 
+      {
+        switch (aCommand) {
+          case "cmd_copy":
+            return true;
+          default:
+            return false;
+        }
+      },
+
+      isCommandEnabled: function (aCommand)
+      {
+        switch (aCommand) {
+          case "cmd_copy":
+            return isItemSelected();
+          default:
+            return false;
+        }
+      },
+
+      doCommand: function (aCommand)
+      {
+        switch (aCommand) {
+          case "cmd_copy":
+            copyItemToClipboard();
+            break;
+          default:
+            break;
+        }
+      },
+
+      onEvent: function (aEvent) 
+      {
+      }
+    }
+
+    function updateCopyMenu()
+    {
+      goUpdateCommand("cmd_copy");
+    }
+
+    function initConsoleOverlay()
+    {
+      window.removeEventListener("load", initConsoleOverlay, false);
+      top.controllers.insertControllerAt(0, ConsoleController);
+
+    }
+
+    window.addEventListener("load", initConsoleOverlay, false);
+  ]]></script>
+
   <commandset id="consoleCommands">
     <commandset id="tasksCommands"/>
     <command id="cmd_quit"/>
   </commandset>
 
   <keyset id="consoleKeys">
     <keyset id="tasksKeys"/>
     <key id="key_copy"/>
(Assignee)

Comment 5

11 years ago
Created attachment 251385 [details] [diff] [review]
Patch 1.1a

Whee. What fun I can't edit a patch it just becomes another comment.
Attachment #251384 - Attachment is obsolete: true

Comment 6

11 years ago
(In reply to comment #3)
>>Nit: correct order is supportsCommand, then isCommandEnabled, then doCommand
>That was the order in the XPFE console.js introduced in Bug 67647. You didn't
>mention anything about the order then.
Touché, but then I wasn't formally reviewing it (note the r=jag).

>>>+      window.removeEventListener("load", initConsoleOverlay, false);
>>Might not have to do this in a load event.
>Won't this leak when the window closes?
Actually I quoted the wrong line; I meant to quote the top.controllers line.
But in answer to your question, all event listeners get unhooked.
(Assignee)

Comment 7

11 years ago
Created attachment 251496 [details] [diff] [review]
Patch 1.1b (Checked into trunk)

1. Change order of members in the controller.
2. Remove the load event listener.
3. Instead of overriding the toolkit console's updateCopyMenu() we call goUpdateCommand('cmd_copy') directly in the onpopupshowing handler attribute.
Attachment #251385 - Attachment is obsolete: true
Attachment #251496 - Flags: review?(neil)

Comment 8

11 years ago
Comment on attachment 251496 [details] [diff] [review]
Patch 1.1b (Checked into trunk)

r+sr=me
Attachment #251496 - Flags: review?(neil) → review+

Comment 9

11 years ago
Comment on attachment 251496 [details] [diff] [review]
Patch 1.1b (Checked into trunk)

Checking in (trunk)
consoleOverlay.xul;
new revision: 1.2; previous revision: 1.1
done
Attachment #251496 - Attachment description: Patch 1.1b → Patch 1.1b (Checked into trunk)
Blocks: 328887

Updated

11 years ago
Depends on: 334997

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

9 years ago
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.