When the user mouses over a CSS property in the styles panel and pressed the command key, it should underline directly so that it's clear that jump-to-definition is available. Right now, you need to move the mouse before the Cmd key press is registered.
<rdar://problem/14521089>
Created attachment 213249 [details] Patch
Comment on attachment 213249 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213249&action=review I think you can remove a bunch of code here now right? If CodeMirrorTokenTracking controller is always tracking when appropriate then: 1. Make CodeMirrorTokenTrackingController.prototype.{start,stop}Tracking private, or roll them into _mouseEntered and _mouseLeft 2. Remove CodeMirrorTokenTrackingController.prototype.tracking and code that uses it - Remove SourceCodeTextEditor.prototype._shouldTrackTokenHovering, _startTrackingTokenHoveringIfNeeded, _stopTrackingTokenHoveringIfNeeded, _debuggerDidPause - Remove registration and callers of _debuggerDidPause The only thing I am unsure of is the bottom two lines of this patch (if we disable jump to symbol tracking by unpressing Cmd, the removal highlighted range). I suspect you can just remove the if tracking branch. I'd like to see a follow up patch removing the dead code! > Source/WebInspectorUI/ChangeLog:22 > + tracking since otherwise pressing the Cmd key over a read-only editor could highlight the previously selected > + candidate of another editor. Is that really true? Each editor gets a separate tokenTrackingController instance, how could a range be highlighted in a separate editor? > Source/WebInspectorUI/UserInterface/Main.js:110 > + window.addEventListener("mousemove", this._mousemoved.bind(this), true); Style: "_mousemoved" => "_mouseMoved"
(In reply to comment #3) > (From update of attachment 213249 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213249&action=review > > I think you can remove a bunch of code here now right? If CodeMirrorTokenTracking controller is always tracking when appropriate then: > > 1. Make CodeMirrorTokenTrackingController.prototype.{start,stop}Tracking private, or roll them into _mouseEntered and _mouseLeft > 2. Remove CodeMirrorTokenTrackingController.prototype.tracking and code that uses it > - Remove SourceCodeTextEditor.prototype._shouldTrackTokenHovering, _startTrackingTokenHoveringIfNeeded, _stopTrackingTokenHoveringIfNeeded, _debuggerDidPause > - Remove registration and callers of _debuggerDidPause > > The only thing I am unsure of is the bottom two lines of this patch (if we disable jump to symbol tracking by unpressing Cmd, the removal highlighted range). I suspect you can just remove the if tracking branch. > > I'd like to see a follow up patch removing the dead code! Most of the code I added ends up being specific to the NonSymbolTokens mode, I should see if I could shave off some code for the other mode too. > > Source/WebInspectorUI/ChangeLog:22 > > + tracking since otherwise pressing the Cmd key over a read-only editor could highlight the previously selected > > + candidate of another editor. > > Is that really true? Each editor gets a separate tokenTrackingController instance, how could a range be highlighted in a separate editor? Because the monitoring of key modifier changes is shared through all text editor instances. There's probably another way to deal with this though. > > Source/WebInspectorUI/UserInterface/Main.js:110 > > + window.addEventListener("mousemove", this._mousemoved.bind(this), true); > > Style: "_mousemoved" => "_mouseMoved" Thanks for spotting that.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 213249 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=213249&action=review > > > > > Source/WebInspectorUI/ChangeLog:22 > > > + tracking since otherwise pressing the Cmd key over a read-only editor could highlight the previously selected > > > + candidate of another editor. > > > > Is that really true? Each editor gets a separate tokenTrackingController instance, how could a range be highlighted in a separate editor? > > Because the monitoring of key modifier changes is shared through all text editor instances. There's probably another way to deal with this though. But shouldn't your mouse only be over one editor, so only that editor will have a candidate / update. Maybe this is indicative of a bug? Maybe on mouseLeft we need to clear something? It already clears the tracking state, so many something should respect the tracking state?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 213249 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=213249&action=review > > > > > > > Source/WebInspectorUI/ChangeLog:22 > > > > + tracking since otherwise pressing the Cmd key over a read-only editor could highlight the previously selected > > > > + candidate of another editor. > > > > > > Is that really true? Each editor gets a separate tokenTrackingController instance, how could a range be highlighted in a separate editor? > > > > Because the monitoring of key modifier changes is shared through all text editor instances. There's probably another way to deal with this though. > > But shouldn't your mouse only be over one editor, so only that editor will have a candidate / update. > > Maybe this is indicative of a bug? Maybe on mouseLeft we need to clear something? It already clears the tracking state, so many something should respect the tracking state? Correct! I'll fix this in an updated patch coming up.
Created attachment 213357 [details] Patch
Comment on attachment 213357 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213357&action=review r=me, thanks for cleaning up the dead code! > Source/WebInspectorUI/ChangeLog:46 > + "mouseleave" event hanling when we're in the "enabled" state. Additionally, the public Typo: "hanling" => "handling"
Created attachment 213398 [details] Patch for landing
(In reply to comment #8) > (From update of attachment 213357 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213357&action=review > > r=me, thanks for cleaning up the dead code! My pleasure, thanks for the thoughtful and thorough review! > > Source/WebInspectorUI/ChangeLog:46 > > + "mouseleave" event hanling when we're in the "enabled" state. Additionally, the public > > Typo: "hanling" => "handling" Will be fixed in commit.
Comment on attachment 213398 [details] Patch for landing Clearing flags on attachment: 213398 Committed r156923: <http://trac.webkit.org/changeset/156923>
All reviewed patches have been landed. Closing bug.