Closed Bug 366901 Opened 14 years ago Closed 14 years ago

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

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: philip.chee)

References

()

Details

Attachments

(1 file, 3 obsolete files)

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.
Attached patch Patch 1.0 (obsolete) — Splinter Review
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 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.
Attached patch Patch 1.1 (obsolete) — Splinter Review
> (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)
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"/>
Attached patch Patch 1.1a (obsolete) — Splinter Review
Whee. What fun I can't edit a patch it just becomes another comment.
Attachment #251384 - Attachment is obsolete: true
(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.
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 on attachment 251496 [details] [diff] [review]
Patch 1.1b (Checked into trunk)

r+sr=me
Attachment #251496 - Flags: review?(neil) → review+
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)
Depends on: 334997
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.