WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
23849
Keyboard shortcuts needed in Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=23849
Summary
Keyboard shortcuts needed in Web Inspector
Daniel Gackle
Reported
2009-02-09 10:29:05 PST
It would be a big help to developers working in Web Inspector if the debugger allowed for keyboard shortcuts - having to use the mouse to click through every line is a significant drag (though the debugger works great otherwise). The standard shortcuts that I'm familiar with, and which Firebug also uses, are: F10 - step over F11 - step into Shift+F10 - step out F5 or F8 - continue F9 - toggle breakpoint
Attachments
Patch adding debugger and call stack navigation shortcuts.
(10.62 KB, patch)
2009-06-26 01:55 PDT
,
Yury Semikhatsky
no flags
Details
Formatted Diff
Diff
Patch adding debugger and call stack navigation shortcuts(with copyright fix).
(10.92 KB, patch)
2009-06-26 02:02 PDT
,
Yury Semikhatsky
no flags
Details
Formatted Diff
Diff
Patch adding debugger and call stack navigation shortcuts.
(11.40 KB, patch)
2009-06-26 02:10 PDT
,
Yury Semikhatsky
no flags
Details
Formatted Diff
Diff
Add Firebug-compatible debugger and stack navigation shortcuts.
(12.47 KB, patch)
2009-06-29 02:38 PDT
,
Yury Semikhatsky
timothy
: review-
Details
Formatted Diff
Diff
Previous patch with Timothy's comments addressed.
(13.03 KB, patch)
2009-06-30 02:23 PDT
,
Yury Semikhatsky
timothy
: review-
Details
Formatted Diff
Diff
Replaced the tabs with spaces.
(13.04 KB, patch)
2009-06-30 23:43 PDT
,
Yury Semikhatsky
timothy
: review+
Details
Formatted Diff
Diff
Change 'Continue' shortcut from F5 to F8
(1.38 KB, patch)
2009-07-06 00:45 PDT
,
Yury Semikhatsky
timothy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Yury Semikhatsky
Comment 1
2009-06-26 01:55:16 PDT
Created
attachment 31920
[details]
Patch adding debugger and call stack navigation shortcuts.
Francois Botha
Comment 2
2009-06-26 02:01:26 PDT
In Firebug, F8 is used for Continue. F5 causes the page to reload (On my Windows XP, Fx 3.5 system at least). Therefore, my preference is for F8 instead of F5 for Continue.
Yury Semikhatsky
Comment 3
2009-06-26 02:02:53 PDT
Created
attachment 31921
[details]
Patch adding debugger and call stack navigation shortcuts(with copyright fix).
Yury Semikhatsky
Comment 4
2009-06-26 02:10:31 PDT
Created
attachment 31922
[details]
Patch adding debugger and call stack navigation shortcuts.
Yury Semikhatsky
Comment 5
2009-06-26 05:29:41 PDT
Comment on
attachment 31922
[details]
Patch adding debugger and call stack navigation shortcuts. The patch needs to be changed to work properly on all platforms. Marking it as obsolete.
Yury Semikhatsky
Comment 6
2009-06-29 02:38:17 PDT
Created
attachment 32002
[details]
Add Firebug-compatible debugger and stack navigation shortcuts. On Mac most functional keys are bound as OS shortcuts which means they cannot be used as the debugger shortcuts. To overcome this I've added several shortcuts that don't conflict with OS. Those are the keys used in Firebug so users should feel comfortable with them.
Francois Botha
Comment 7
2009-06-29 03:20:41 PDT
Would somebody care to comment on the F5 vs F8 issue? Refer to
comment #2
Yury Semikhatsky
Comment 8
2009-06-29 03:32:43 PDT
(In reply to
comment #7
)
> Would somebody care to comment on the F5 vs F8 issue? Refer to
comment #2
>
If focus is on the sidebar pane pressing F5 should just continue execution(as long as this patch is applied).
Francois Botha
Comment 9
2009-06-29 04:25:28 PDT
I'm aware that F5 will not refresh the page *in Chrome*. But my point is that it *does* refresh the page in Firefox + Firebug. So, if you want to be *consistent*, surely F8 is the right choice?
Yury Semikhatsky
Comment 10
2009-06-29 04:49:49 PDT
(In reply to
comment #9
)
> I'm aware that F5 will not refresh the page *in Chrome*. But my point is that > it *does* refresh the page in Firefox + Firebug. So, if you want to be > *consistent*, surely F8 is the right choice? >
It looks to me as a bug in Firebug and I'd rather avoid repeating it in WebKit.
Timothy Hatcher
Comment 11
2009-06-29 09:58:21 PDT
Comment on
attachment 32002
[details]
Add Firebug-compatible debugger and stack navigation shortcuts.
> + this.shortcuts_ = {};
Why does this end with an underscore? THis dosen't match any of our naming conventions. Just remove the underscore or use a underscore prefix if the property is "private".
> + handleKeyEvent: function(event) {
Open curly brace should be on the next line.
> + var shortcut = WebInspector.KeyboardShortcut.makeKeyFromEvent(event); > + var handler = this.shortcuts_[shortcut]; > + if (handler) { > + handler(event); > + event.preventDefault(); > + event.handled = true; > + } > + },
Use 4 space indents.
> + _selectNextCallFrameOnStack: function() {
Open curly brace should be on the next line.
> + _selectPreviousCallFrameOnStack: function() {
Open curly brace should be on the next line.
> + _selectedPlacardByIndex: function(index) {
Open curly brace should be on the next line.
> + _selectedCallFrameIndex: function() {
Open curly brace should be on the next line.
> + if (placard.callFrame === this._selectedCallFrame) { > + return i; > + }
No need for the curly brances for one line.
> +WebInspector.KeyboardShortcut.Modifiers = { > + NONE: 0, > + SHIFT: 1, > + CTRL: 2, > + ALT: 4, > + META: 8 > +};
No need for the semicolon. I would also like to see these be CamelCase and spelled out.
> +WebInspector.KeyboardShortcut.KeyCodes = { > + ESC: 27, > + SPACE: 32, > + PAGE_UP: 33, // also NUM_NORTH_EAST > + PAGE_DOWN: 34, // also NUM_SOUTH_EAST > + END: 35, // also NUM_SOUTH_WEST > + HOME: 36, // also NUM_NORTH_WEST > + LEFT: 37, // also NUM_WEST > + UP: 38, // also NUM_NORTH > + RIGHT: 39, // also NUM_EAST > + DOWN: 40, // also NUM_SOUTH > + F1: 112, > + F2: 113, > + F3: 114, > + F4: 115, > + F5: 116, > + F6: 117, > + F7: 118, > + F8: 119, > + F9: 120, > + F10: 121, > + F11: 122, > + F12: 123, > + SEMICOLON: 186, > + COMMA: 188, > + PERIOD: 190, > + SLASH: 191, > + APOSTROPHE: 192, > + SINGLE_QUOTE: 222, > +};
Same here. CamelCase and spelled out for the non function keys.
> +WebInspector.KeyboardShortcut.makeKey = function(keyCode, opt_modifiers) {
Open curly brace should be on the next line. The opt_modifiers parameter should be spelled out and camelCase instead of an underscore.
> + for (var i = 1; i < arguments.length; i++) { > + modifiers |= arguments[i]; > + }
No need for the curly brances for one line.
> +WebInspector.KeyboardShortcut.makeKeyFromCodeAndModifiers_ = function(keyCode, modifiers) { > + return (keyCode & 255) | (modifiers << 8); > +};
The underscore should be a prefix if this is a "private" function.
> + this.shortcuts_ = {};
Use an underscore prefix or drop the underscore.
> + handleKeyEvent: function(event) {
Open curly brace should be on the next line.
> + var shortcut = WebInspector.KeyboardShortcut.makeKeyFromEvent(event); > + var handler = this.shortcuts_[shortcut]; > + if (handler) { > + handler(event); > + event.preventDefault(); > + event.handled = true; > + } else { > + this.sidebarPanes.callstack.handleKeyEvent(event); > + } > + },
Use 4 space indent. r- purely on code style. Functionally correct!
Timothy Hatcher
Comment 12
2009-06-29 10:01:28 PDT
It would also be better to use keyIdentifier instead of keyCode (e.g. event.keyIdentifier === "Right").
Yury Semikhatsky
Comment 13
2009-06-30 02:23:05 PDT
Created
attachment 32044
[details]
Previous patch with Timothy's comments addressed. I've fixed all of the pointed issues(sorry, it's hard to switch between two different style guides) except for the semicolons. If you don't mind I'd rather have each expression ending with semicolon explicitly to avoid possible nasty errors happening when semicolon is accidentally omitted between two expressions like in the following example: var x = { 'i': 1, 'j': 2 } // 2. Trying to do one thing on IE and another on FF. // I know you'd never write code like this, but throw me a bone. [normalVersion, ffVersion][isIE]();
Yury Semikhatsky
Comment 14
2009-06-30 02:31:42 PDT
(In reply to
comment #12
)
> It would also be better to use keyIdentifier instead of keyCode (e.g. > event.keyIdentifier === "Right"). >
On Windows pressing F10 gives me keyIdentifier F11, pressing F11 gives U+005A. Also for most of the other keys it's something like U+00BF which means it would require introducing constants for those keys anyway. Using uniform numerical constants for keyCode values looks more convenient.
Adam Roben (:aroben)
Comment 15
2009-06-30 06:30:22 PDT
(In reply to
comment #14
)
> On Windows pressing F10 gives me keyIdentifier F11
This sounds like a bug. Could you please file it?
Timothy Hatcher
Comment 16
2009-06-30 13:25:35 PDT
Comment on
attachment 32044
[details]
Previous patch with Timothy's comments addressed. There are tabs in CallStackSidebarPane.js. Otherwise fine.
Yury Semikhatsky
Comment 17
2009-06-30 23:43:19 PDT
Created
attachment 32116
[details]
Replaced the tabs with spaces.
Yury Semikhatsky
Comment 18
2009-06-30 23:56:44 PDT
(In reply to
comment #15
)
> (In reply to
comment #14
) > > On Windows pressing F10 gives me keyIdentifier F11 > > This sounds like a bug. Could you please file it? >
Done:
https://bugs.webkit.org/show_bug.cgi?id=26878
Eric Seidel (no email)
Comment 19
2009-07-01 18:03:39 PDT
Sending WebCore/ChangeLog Sending WebCore/inspector/front-end/CallStackSidebarPane.js Adding WebCore/inspector/front-end/KeyboardShortcut.js Sending WebCore/inspector/front-end/ScriptsPanel.js Sending WebCore/inspector/front-end/WebKit.qrc Sending WebCore/inspector/front-end/inspector.html Transmitting file data ...... Committed revision 45460.
Francois Botha
Comment 20
2009-07-03 03:35:57 PDT
> It looks to me as a bug in Firebug and I'd rather avoid repeating it in WebKit.
Possible bug filed at
http://code.google.com/p/fbug/issues/detail?id=1960
for clarification. If it's not a bug (as I suspect) I hope this patch gets adjusted.
Francois Botha
Comment 21
2009-07-04 00:20:37 PDT
Firebug developers confirm that the Continue button is F8, not F5.
http://code.google.com/p/fbug/issues/detail?id=1960
So, if you want to be consistent, emulate Firebug. Emulating Visual Studio makes less sense because web developers use Firebug a lot more than VS.
Yury Semikhatsky
Comment 22
2009-07-06 00:45:33 PDT
Created
attachment 32287
[details]
Change 'Continue' shortcut from F5 to F8
Pavel Feldman
Comment 23
2009-07-07 10:57:55 PDT
Sending WebCore/ChangeLog Sending WebCore/inspector/front-end/ScriptsPanel.js Transmitting file data .. Committed revision 45591.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug