Bug 37579

Summary: Web Inspector: Ctrl-L (Clear Console) does nothing on Windows
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
[PATCH] Trivial fix
none
[PATCH] Comments addressed none

Description Alexander Pavlov (apavlov) 2010-04-14 09:18:39 PDT
The console is not cleared on non-Mac platforms when Ctrl-L is hit in the text prompt.
Comment 1 Alexander Pavlov (apavlov) 2010-04-14 09:23:55 PDT
Created attachment 53337 [details]
[PATCH] Trivial fix
Comment 2 Joseph Pecoraro 2010-04-14 09:42:31 PDT
Comment on attachment 53337 [details]
[PATCH] Trivial fix

I don't understand how this fixes Ctrl "L" on Windows.

> 1      var clearConsoleHandler = this.requestClearMessages.bind(this);
> 2 
> 3      shortcut = WebInspector.KeyboardShortcut.makeKey("k", WebInspector.KeyboardShortcut.Modifiers.Meta);
> 4 -    this._shortcuts[shortcut] = clearConsoleHandler;
> 5 +    this._shortcuts[shortcut] = this.requestClearMessages.bind(this);
> 6      this._shortcuts[shortcut].isMacOnly = true;
> 7      shortcut = WebInspector.KeyboardShortcut.makeKey("l", WebInspector.KeyboardShortcut.Modifiers.Ctrl);
> 8      this._shortcuts[shortcut] = clearConsoleHandler;


Line 1 shows that the clearConsoleHandler variable already has the requestClearMessages bound to this already. So the change doesn't appear to do anything. Also, the change is made to the Mac only <Cmd>K clear shortcut, not to the Windows <Ctrl>L part. Is there something I am missing?
Comment 3 Joseph Pecoraro 2010-04-14 09:46:04 PDT
Comment on attachment 53337 [details]
[PATCH] Trivial fix

Ahhhh, I think I got it. The "isMacOnly" property was being set on the shared bound function, and so all other cases (like Windows) see the isMacOnly property and don't run. Thats unfortunate.

r=me if you Provide a comment explaining this (because its tricky). You might even want to reorder the code slightly if that helps make it clearer as well.
Comment 4 Alexander Pavlov (apavlov) 2010-04-14 10:02:34 PDT
Created attachment 53342 [details]
[PATCH] Comments addressed
Comment 5 Joseph Pecoraro 2010-04-14 10:10:25 PDT
Comment on attachment 53342 [details]
[PATCH] Comments addressed

Excellent. Thanks!
Comment 6 WebKit Commit Bot 2010-04-14 11:18:43 PDT
Comment on attachment 53342 [details]
[PATCH] Comments addressed

Clearing flags on attachment: 53342

Committed r57590: <http://trac.webkit.org/changeset/57590>
Comment 7 WebKit Commit Bot 2010-04-14 11:18:50 PDT
All reviewed patches have been landed.  Closing bug.