Closed
Bug 832984
Opened 12 years ago
Closed 12 years ago
Ctrl + Shift + K shortcut for the Web Console stopped working (sometimes)
Categories
(DevTools :: Console, defect, P1)
Tracking
(firefox20+ verified, firefox21 verified)
VERIFIED
FIXED
Firefox 21
People
(Reporter: djc, Assigned: paul)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jwalker
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In 20.0a2 (2013-01-17).
Comment 1•12 years ago
|
||
Sounds like a toolbox issue.
Assignee | ||
Comment 2•12 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•12 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•12 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 | ||
Comment 5•12 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•12 years ago
|
||
You need to remove the <keyset> and reinsert it into the document.
Flags: needinfo?(enndeakin)
Updated•12 years ago
|
Blocks: DevToolsPaperCuts
Assignee | ||
Comment 7•12 years ago
|
||
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 | ||
Comment 8•12 years ago
|
||
typo
Attachment #704860 -
Attachment is obsolete: true
Attachment #704860 -
Flags: review?(jwalker)
Attachment #704861 -
Flags: review?(jwalker)
Assignee | ||
Comment 9•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=1ad733830171
Comment 10•12 years ago
|
||
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•12 years ago
|
||
addressed Joe's comment.
Updated•12 years ago
|
Attachment #704888 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Comment 12•12 years ago
|
||
We need that in Firefox 20.
Assignee | ||
Updated•12 years ago
|
Attachment #704861 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 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?
Comment 14•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/8d85fac98a8c
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Updated•12 years ago
|
Comment 15•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8d85fac98a8c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21
Updated•12 years ago
|
Comment 16•12 years ago
|
||
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+
Comment 18•12 years ago
|
||
Verified as fixed on FF 20.b1: Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0
Comment 19•12 years ago
|
||
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
Updated•11 years ago
|
No longer blocks: DevToolsPaperCuts
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•