(Devtools) Browser Console keyboard shortcut does not work

RESOLVED FIXED in seamonkey2.49

Status

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: kevink9876543, Assigned: kevink9876543)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.49
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.48 fixed, seamonkey2.49esr fixed, seamonkey2.50 fixed)

Details

(Whiteboard: [devtools])

Attachments

(4 attachments, 4 obsolete attachments)

Assignee

Description

3 years ago
OS: Linux x86_64 (but Windows users see this as well)

Current SeaMonkey Nightly sets Ctrl-Shift-J for both Browser Console & Error Console, but Error Console takes precedence, leaving Browser Console without a keyboard shortcut.

Prior version had set Ctrl-Shift-Z for Browser Console, but that keyboard shortcut did not work either (appears to did nothing), probably because that's the keyboard shortcut for "Redo".

Comment 1

3 years ago
A recent change in the devtools code is that all menu items are dynamically generated so the keyboard short cut keys are overriding our XUL defined shortcuts so we need to override /devtools/client/locales/en-US/menus.properties with our own.

Plan of action:
1. Create a menus.properties file and put it in suite/locales/en-US/chrome/browser/
2. In suite/locales/jar.mn add an override:
   override chrome://devtools/locale/menus.properties chrome://navigator/locale/menus.properties
3. You also need to add a line in that jar.mn file like:
   locale/@AB_CD@/navigator/menus.properties (%chrome/browser/menus.properties)
Assignee

Updated

3 years ago
Assignee: nobody → kevink9876543
Status: NEW → ASSIGNED
Assignee

Comment 2

3 years ago
Posted patch bug1265485.patch (obsolete) — Splinter Review
Couple notes about this patch:
1) the menus.properties was copied from mozilla/devtools/client/locales/en-US then edited
2) apparently we can't use just any unassigned hotkey here - I had initially tried to use Ctrl-Shift-Y, but it did nothing.  So went with Ctrl-Shift-O which WFM.
Attachment #8742902 - Flags: review?(philip.chee)
Assignee

Updated

3 years ago
Assignee: kevink9876543 → nobody
Status: ASSIGNED → NEW
http://hg.mozilla.org/releases/comm-aurora/rev/70c2c9d7851e#l8.20
> <!ENTITY browserConsoleCmd.commandkey    "Ζ">

I have noticed the "Ζ"(0x396) is the Greek alphabet, is not latin "Z" (0x5a).
It shouldn't be used in en-US locale.
Flags: needinfo?(philip.chee)

Comment 4

3 years ago
(In reply to Masahiko Imanaka [:marsf] from comment #3)
> http://hg.mozilla.org/releases/comm-aurora/rev/70c2c9d7851e#l8.20
> > <!ENTITY browserConsoleCmd.commandkey    "Ζ">
> 
> I have noticed the "Ζ"(0x396) is the Greek alphabet, is not latin "Z" (0x5a).
> It shouldn't be used in en-US locale.

Congratulations! You win a no-prize!
http://goodcomics.comicbookresources.com/wp-content/uploads/2016/02/no-prize.jpg
Flags: needinfo?(philip.chee)

Updated

3 years ago
See Also: → 1289981

Updated

3 years ago
Blocks: smdevtools

Updated

3 years ago
See Also: → 1289982

Comment 5

3 years ago
Ctrl+Shift+J, which worked with Firefox 47, opens page source starting from version 48.
This should not be assigned to SeaMonkey.

Updated

3 years ago
Whiteboard: [devtools]
Assignee

Updated

3 years ago
Attachment #8742902 - Flags: review?(philip.chee)
Assignee

Updated

3 years ago
Assignee: nobody → kevink9876543
Assignee

Comment 6

3 years ago
With Error Console being part of comm-* now, here's a patch that instead makes *its* keyboard shortcut Ctrl-Shift-O.  This frees up Ctrl-Shift-J for the Browser Console.

Tested in 2.46 and current comm-central.
Attachment #8742902 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8802736 - Flags: review?(iann_bugzilla)

Comment 7

3 years ago
Comment on attachment 8802736 [details] [diff] [review]
bug1265485.patch 2

r/a=me
Attachment #8802736 - Flags: review?(iann_bugzilla) → review+

Updated

3 years ago
Status: NEW → ASSIGNED
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 8

3 years ago
https://hg.mozilla.org/comm-central/rev/a1ac8eae4234ff0280aab2ee5fe8175598d1ef57
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.49
Assignee

Comment 9

3 years ago
Comment on attachment 8802736 [details] [diff] [review]
bug1265485.patch 2

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: Can't access Browser Console by keyboard shortcut
Testing completed (on m-c, etc.): landed on c-c, tested locally w/ 2.46
Risk to taking this patch (and alternatives if risky): low risk
String changes made by this patch: none
Attachment #8802736 - Flags: approval-comm-aurora?

Comment 10

3 years ago
(In reply to kevink9876543 from comment #9)
> Comment on attachment 8802736 [details] [diff] [review]
> bug1265485.patch 2
> 
> [Approval Request Comment]
> Regression caused by (bug #): 
> User impact if declined: Can't access Browser Console by keyboard shortcut
> Testing completed (on m-c, etc.): landed on c-c, tested locally w/ 2.46
> Risk to taking this patch (and alternatives if risky): low risk
> String changes made by this patch: one string change. Only an accelerator key
> which each locale translates differently anyway.
Flags: needinfo?(iann_bugzilla)
Assignee

Comment 11

3 years ago
Comment on attachment 8802736 [details] [diff] [review]
bug1265485.patch 2

Moving approval request to comm-beta now that the 52 cycle has become aurora.
Attachment #8802736 - Flags: approval-comm-aurora? → approval-comm-beta?

Updated

3 years ago
Flags: needinfo?(iann_bugzilla)
Attachment #8802736 - Flags: approval-comm-beta? → approval-comm-beta+
https://hg.mozilla.org/releases/comm-beta/rev/a2dfa6e67d4f714f0e6f7ac3aabde1505f4bbb61

f- for IanN :) Tabs in the file and patch at the exact location. But already in c-c so...

Comment 13

2 years ago
Fix causes "Bug 1326258 - Paste As Quotation shortcut conflicts with Error Console shortcut"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 14

2 years ago
Has impact at "Bug 1284288 - shortcuts.xhtml: adapt 'Error Console', 'DOM Inspector' and new 'FF DevTools' related contents"

Updated

2 years ago
Blocks: 1326258
Assignee

Comment 15

2 years ago
Oops, sorry about that!  What keyboard shortcut would you suggest using for Error Console?
Assignee

Comment 16

2 years ago
Posted patch bug1265485-fix.patch (obsolete) — Splinter Review
Per discussion on irc with RattyAway, shortcut is changed to Accel-Alt-J
Assignee

Updated

2 years ago
Attachment #8823060 - Flags: review?(frgrahl)
Comment on attachment 8823060 [details] [diff] [review]
bug1265485-fix.patch

Kevin,

it works but by changing the modifiers you created a change impacting all languages. Localizer have no way to actually find this change.

Could you rename commandkey to commandkey2 and check if this still works. I will r+ it then.
Attachment #8823060 - Flags: review?(frgrahl) → review-
Assignee

Comment 18

2 years ago
Thanks.
Attachment #8823060 - Attachment is obsolete: true
Attachment #8824633 - Flags: review?(frgrahl)
Comment on attachment 8824633 [details] [diff] [review]
addressed review comments

Looks good. Thanks. Will check it in later if you don't have access.
Attachment #8824633 - Flags: review?(frgrahl) → review+
Assignee

Updated

2 years ago
Keywords: checkin-needed
Because I was trusted with reviewing it I just added a+ too.

https://hg.mozilla.org/comm-central/rev/78c415cf613ecfd5cbedcb580d1cf5273981f9a9

Should this go into a possible 2.49esr?
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago2 years ago
Flags: needinfo?(philip.chee)
Resolution: --- → FIXED
Assignee

Comment 21

2 years ago
Comment on attachment 8824633 [details] [diff] [review]
addressed review comments

[Approval Request Comment]

Regression caused by (bug #): this bug

User impact if declined: Cannot use "Paste as Quotation" keyboard shortcut

Testing completed (on m-c, etc.): landed in c-c, locally tested on c-a

Risk to taking this patch (and alternatives if risky): low risk

String changes made by this patch:
1) errorConsoleCmd.commandkey is renamed to errorConsoleCmd.commandkey2
2) keyboard shortcut modifiers for Error Console were changed, affecting all locales
Attachment #8824633 - Flags: approval-comm-beta?
Attachment #8824633 - Flags: approval-comm-aurora?

Updated

2 years ago
Duplicate of this bug: 1326258
Assignee

Updated

2 years ago
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 23

2 years ago
Comment on attachment 8824633 [details] [diff] [review]
addressed review comments

> [Approval Request Comment]

> String changes made by this patch:
> 1) errorConsoleCmd.commandkey is renamed to errorConsoleCmd.commandkey2
> 2) keyboard shortcut modifiers for Error Console were changed, affecting all
> locales
String changes are a  big NO-NO for comm-aurora and comm-beta and comm-release. Localizers really hate being blindsided by last late breaking changes. So given that the Browser Console can still be open via the menu this is a r- a-.

Sorry but this will have to ride the trains.
Flags: needinfo?(philip.chee)
Attachment #8824633 - Flags: approval-comm-beta?
Attachment #8824633 - Flags: approval-comm-beta-
Attachment #8824633 - Flags: approval-comm-aurora?
Attachment #8824633 - Flags: approval-comm-aurora-
As an alternative because the current key clashes with another key in mail composition how about removing it from c-a and c-b?

Comment 25

2 years ago
(In reply to Frank-Rainer Grahl from comment #24)
> As an alternative because the current key clashes with another key in mail
> composition how about removing it from c-a and c-b?
Yes Please.
Assignee

Comment 26

2 years ago
(In reply to Philip Chee from comment #25)
> (In reply to Frank-Rainer Grahl from comment #24)
> > As an alternative because the current key clashes with another key in mail
> > composition how about removing it from c-a and c-b?
> Yes Please.

If I'm understanding this correctly, here's a patch for that.
Attachment #8826890 - Flags: review?(frgrahl)
Followup whitespace chleanup for c-c as discussed on irc (or new bug?)
Attachment #8829103 - Flags: review?(philip.chee)
Kevin,

just had time to look at the patch. Sorry it took so long.

You missed a line with the original key so that the assigned key wouldn't interfere anywhere. Because of the removal tasksOverlay.dtd does not need a change. The unused old key can ride the train.

Becuase it was a small trivial change I patched it up myself and redirecting to Ratty also for approval. Hope this is ok with you. Tested with 2.48.
Attachment #8826890 - Attachment is obsolete: true
Attachment #8826890 - Flags: review?(frgrahl)
Attachment #8829104 - Flags: review?(philip.chee)
Attachment #8829104 - Flags: approval-comm-beta?
Attachment #8829104 - Flags: approval-comm-aurora?
Assignee

Comment 29

2 years ago
Sorry, I'm still confused.  Why remove this line? -
>  accesskey="&errorConsoleCmd.accesskey;"

The key that was changed in this bug is errorConsoleCmd.*commandkey*, not errorConsoleCmd.*accesskey*.  The latter is untouched by the patches in this bug.

What am I missing?
Flags: needinfo?(frgrahl)
Sorry you are right. Need to concentrate more... Confused accesskey with the other key. r+ because it essentially your first patch with the dtd part removed.

IanN or Ratty need to approve it.
Attachment #8829104 - Attachment is obsolete: true
Attachment #8829104 - Flags: review?(philip.chee)
Attachment #8829104 - Flags: approval-comm-beta?
Attachment #8829104 - Flags: approval-comm-aurora?
Flags: needinfo?(frgrahl)
Attachment #8829157 - Flags: review+
Attachment #8829157 - Flags: approval-comm-beta?
Attachment #8829157 - Flags: approval-comm-aurora?

Comment 31

2 years ago
Comment on attachment 8829157 [details] [diff] [review]
bug1265485-aurora-beta-fix-V2a.patch

a=me for comm-aurora and comm-beta
Attachment #8829157 - Flags: approval-comm-beta?
Attachment #8829157 - Flags: approval-comm-beta+
Attachment #8829157 - Flags: approval-comm-aurora?
Attachment #8829157 - Flags: approval-comm-aurora+

Comment 32

2 years ago
Comment on attachment 8829103 [details] [diff] [review]
1265485-whitespace-cleanup.patch

> Followup whitespace chleanup for c-c as discussed on irc (or new bug?)
A new bug would be nice.

> -<!ENTITY editorCmd.label					"Composer">
> -<!ENTITY editorCmd.accesskey				"c">
> -<!ENTITY editorCmd.commandkey				"4">
> +<!ENTITY editorCmd.label "Composer">
> +<!ENTITY editorCmd.accesskey "c">
> +<!ENTITY editorCmd.commandkey "4">
I would have accepted vertically aligning the "values" but this works too.
Attachment #8829103 - Flags: review?(philip.chee) → review+
You need to log in before you can comment on or make changes to this bug.