Closed Bug 366479 Opened 13 years ago Closed 13 years ago

[mac] Copy command is broken in the JS console

Categories

(Firefox :: Menus, defect, P2)

PowerPC
macOS
defect

Tracking

()

RESOLVED FIXED
Firefox 3 alpha3

People

(Reporter: sdwilsh, Assigned: mano)

References

Details

(Keywords: dogfood)

Attachments

(1 file, 7 obsolete files)

When trying to paste with the keyboard shortcut, I just get a beep.  Pasting from the context menu works just fine.
Works: my last build from before Cocoa widgets were turned on.
Fails: my first build after Cocoa widgets were turned on.
Works: a build from ten minutes ago, with --enable-default-toolkit=mac

To Cocoa with you, bug!
Assignee: nobody → joshmoz
Component: Error Console → Widget: Cocoa
Product: Firefox → Core
QA Contact: javascript.console → cocoa
(In reply to comment #1)
> Works: my last build from before Cocoa widgets were turned on.
> Fails: my first build after Cocoa widgets were turned on.
> Works: a build from ten minutes ago, with --enable-default-toolkit=mac
> 
> To Cocoa with you, bug!

The strange thing is that it works in other places just fine.
Assignee: joshmoz → mano
So the fix for bug 291276 doesn't play nice with cocoa widget, sigh.

The edit menu items was were always disabled in the JS-console (and few other windows which only have menus on mac), but the key still worked.
Status: NEW → ASSIGNED
Component: Widget: Cocoa → Menus
Priority: -- → P2
Product: Core → Firefox
QA Contact: cocoa → menus
Target Milestone: --- → Firefox 3 alpha2
Attached patch patch (obsolete) — Splinter Review
Attachment #252686 - Flags: review?(gavin.sharp)
Summary: CMD+V does not paste any text in error console → [mac] Edit menu items are disabled in non-browser windows (was: CMD+V does not paste any text in error console)
Attached patch patch (obsolete) — Splinter Review
easier now that bug 366992 is fixed.
Attachment #252686 - Attachment is obsolete: true
Attachment #255162 - Flags: review?(gavin.sharp)
Attachment #252686 - Flags: review?(gavin.sharp)
Flags: in-testsuite-
Whiteboard: [a3]
Target Milestone: Firefox 3 alpha2 → Firefox 3
Whiteboard: [a3]
Target Milestone: Firefox 3 → Firefox 3 alpha3
Attached patch patch (obsolete) — Splinter Review
Attachment #255162 - Attachment is obsolete: true
Attachment #255710 - Flags: review?(gavin.sharp)
Attachment #255162 - Flags: review?(gavin.sharp)
Comment on attachment 255710 [details] [diff] [review]
patch

Hrm, the js console should have a cmd_copy command regardless of macBrowserOverlay (we still need the browserMountPoints.inc for other windows).
Attachment #255710 - Flags: review?(gavin.sharp)
Attached patch patch (obsolete) — Splinter Review
- Don't remove cmd_copy from console.xul (add it via editMenuOverlay). I did keep the macBrowserOverlay part though for the reason mentioned in the previous comment.
- Remove the keypress handler, we get one for free from browser-base.inc.
Attachment #255710 - Attachment is obsolete: true
Attachment #255739 - Flags: review?(gavin.sharp)
Attached patch patch (obsolete) — Splinter Review
see above :-/
Attachment #255739 - Attachment is obsolete: true
Attachment #255740 - Flags: review?(gavin.sharp)
Attachment #255739 - Flags: review?(gavin.sharp)
Attached patch don't break suiterunner (obsolete) — Splinter Review
Attachment #255741 - Flags: superreview?(neil)
Attachment #255741 - Flags: review?(neil)
Comment on attachment 255741 [details] [diff] [review]
don't break suiterunner

Note that for some unknown reason I get an xpconnect scope assertion when I close the console window with these patches applied.
Attachment #255741 - Flags: superreview?(neil)
Attachment #255741 - Flags: superreview+
Attachment #255741 - Flags: review?(neil)
Attachment #255741 - Flags: review+
hm, this seems to be working now - not sure what fixed it.
I incidentally fixed the original issue in bug 368918.

This broke the copy command in the js console (on mac), thus morphing this bug to fix just that.
Summary: [mac] Edit menu items are disabled in non-browser windows (was: CMD+V does not paste any text in error console) → [mac] Copy command is broken in the JS console
Attached patch patch (obsolete) — Splinter Review
Beltzner: this removes Accel+C from the context menu of items in the error console. I figured we should do so regardless of this bug, unless you disagree ;)
Attachment #255740 - Attachment is obsolete: true
Attachment #256396 - Flags: ui-review?(beltzner)
Attachment #256396 - Flags: review?(gavin.sharp)
Attachment #255740 - Flags: review?(gavin.sharp)
Comment on attachment 256396 [details] [diff] [review]
patch

>Index: toolkit/components/console/content/console.js

> function copyItemToClipboard()
> {
>   gConsole.copySelectedItem();
> }

Looks like this function is now unused (assuming you also land the suite patch).

>Index: toolkit/components/console/content/consoleBindings.xml

>       <property name="selectedItem">
>         <getter>return this.mSelectedItem</getter>
>         <setter><![CDATA[

>+          // Update edit commands
>+          window.updateCommands("focus");

I'm not sure I understand why this is needed. Won't the global focus controller take care of updating the focus commands automatically? I guess it might only do that when the window itself is focused and has no current focused element. Is that not sufficient in this case?
No focus or blur event is triggered when the hbox is already "focused", i.e. when clicking on the empty area before selecting a row.
Comment on attachment 256396 [details] [diff] [review]
patch

Ok, r=me with or without the updateCommands call, if you file a followup about the edgecases we talked about yesterday (command not being updated properly when tabbing out of the textfield, clearing items from the console, switching panes, etc). Don't forget the first part of comment 16, either.

I don't think this needs ui-review, for what it's worth, since we're just making this menu consistent with all the other edit context menus.
Attachment #256396 - Flags: review?(gavin.sharp) → review+
Attached patch as checked inSplinter Review
mozilla/toolkit/components/console/content/console.js 1.6
mozilla/toolkit/components/console/content/console.xul 1.11
mozilla/toolkit/components/console/content/consoleBindings.xml 1.18
Attachment #255741 - Attachment is obsolete: true
Attachment #256396 - Attachment is obsolete: true
Attachment #256396 - Flags: ui-review?(beltzner)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 379439
Duplicate of this bug: 399962
You need to log in before you can comment on or make changes to this bug.