If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Ctrl + Shift + K shortcut for the Web Console stopped working (sometimes)

VERIFIED FIXED in Firefox 20

Status

()

Firefox
Developer Tools: Console
P1
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: djc, Assigned: paul)

Tracking

20 Branch
Firefox 21
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox20+ verified, firefox21 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
In 20.0a2 (2013-01-17).
Sounds like a toolbox issue.
(Assignee)

Comment 2

5 years ago
@Dirkjan, a couple of questions:
- do you see the web console menu entry in the Web Developer menu?
- if you restart Firefox, does it re-work? (serious question)
(Reporter)

Comment 3

5 years ago
Yes, I saw the web console menu entry in the Web Developer menu, and it worked fine.

I'll try restarting Firefox tomorrow, when I'm back at work.

To be sure: this seems to work fine on OS X.
(Assignee)

Comment 4

5 years ago
Ok - this might be an important issue. Keybindings might not be attached correctly at startup. I think it happened to me once. We'll need some help to confirm this bug.
Priority: -- → P1
Summary: Ctrl + Shift + K shortcut for the Web Console stopped working → Ctrl + Shift + K shortcut for the Web Console stopped working (sometimes)
(Assignee)

Updated

5 years ago
Blocks: 830911
(Assignee)

Comment 5

5 years ago
In this loop:

> while true
> do
> firefox -P dev --no-remote #Firefox opens 3 windows from restored session
> done

... after ~15 attempts, I managed to run into this situation. 3 windows. In 2 of them, we can't use ctrl-k (or any dynamically registered keybinding). But menuitems are presents.

Looking at the DOM of the non-working windows, the key is in place.

So apparently, the dynamically-added <key> element is not taken into account.

We create the <key> here:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/framework/gDevTools.jsm#463

Neil, do you see any reason why adding a <key> dynamically would, sometimes, fail?
Flags: needinfo?(enndeakin)

Comment 6

5 years ago
You need to remove the <keyset> and reinsert it into the document.
Flags: needinfo?(enndeakin)
No longer blocks: 830911
Blocks: 831711
(Assignee)

Comment 7

5 years ago
Created attachment 704860 [details] [diff] [review]
v1

So I don't feel like re-attaching the main keyset. So in this patch, I build a devtools specific keyset that we re-attach if necessary.

This appears to work.
Assignee: nobody → paul
Status: NEW → ASSIGNED
Attachment #704860 - Flags: review?(jwalker)
(Assignee)

Comment 8

5 years ago
Created attachment 704861 [details] [diff] [review]
v1.1

typo
Attachment #704860 - Attachment is obsolete: true
Attachment #704860 - Flags: review?(jwalker)
Attachment #704861 - Flags: review?(jwalker)
(Assignee)

Comment 9

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=1ad733830171
Comment on attachment 704861 [details] [diff] [review]
v1.1

Review of attachment 704861 [details] [diff] [review]:
-----------------------------------------------------------------

I think it would be good to have a function instead of the duplicated code?

function(child) {
  let devtoolsKeyset = doc.getElementById("devtoolsKeyset");
  if (!devtoolsKeyset) {
    devtoolsKeyset = doc.createElement("keyset");
    devtoolsKeyset.setAttribute("id", "devtoolsKeyset");
  }
  devtoolsKeyset.appendChild(child);

  // We have to re-attach the <keyset> for the <key>s to be taken
  // into account.
  let mainKeyset = doc.getElementById("mainKeyset");
  mainKeyset.parentNode.insertBefore(devtoolsKeyset, mainKeyset);
}
Attachment #704861 - Flags: review?(jwalker) → review+
(Assignee)

Comment 11

5 years ago
Created attachment 704888 [details] [diff] [review]
v1.2

addressed Joe's comment.
Attachment #704888 - Flags: review+
(Assignee)

Updated

5 years ago
Blocks: 816946
(Assignee)

Updated

5 years ago
Whiteboard: [land-in-fx-team]
(Assignee)

Comment 12

5 years ago
We need that in Firefox 20.
(Assignee)

Updated

5 years ago
Attachment #704861 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Comment on attachment 704888 [details] [diff] [review]
v1.2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature toolbox
User impact if declined: pretty bad. Often, all the devtools shortcut are broken
Testing completed (on m-c, etc.): local only
Risk to taking this patch (and alternatives if risky): Low. But Talos hasn't tested this patch yet. I'm not 100% sure we won't have a small perf regression (building one node dynamically at startup).
String or UUID changes made by this patch: no
Attachment #704888 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/integration/fx-team/rev/8d85fac98a8c
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
status-firefox20: --- → affected
status-firefox21: --- → affected
tracking-firefox20: --- → +
https://hg.mozilla.org/mozilla-central/rev/8d85fac98a8c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21

Updated

5 years ago
status-firefox21: affected → fixed
Comment on attachment 704888 [details] [diff] [review]
v1.2

Approving for aurora uplift, assuming that devs will keep an eye on talos once this lands and report any perf issues.
Attachment #704888 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ca21e77bb5e7
status-firefox20: affected → fixed
Depends on: 839890
Verified as fixed on FF 20.b1:

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0
status-firefox20: fixed → verified
Verified fix on FF 21b1:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0
(20130401192816)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20100101 Firefox/21.0
(20130401192816)
Status: RESOLVED → VERIFIED
status-firefox21: fixed → verified
No longer blocks: 831711
You need to log in before you can comment on or make changes to this bug.