Bug 152218 - Web Inspector: CodeMirrorTokenTrackingController handles symbols in class definitions incorrectly
Summary: Web Inspector: CodeMirrorTokenTrackingController handles symbols in class def...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-12-12 20:12 PST by Matt Baker
Modified: 2015-12-14 10:29 PST (History)
8 users (show)

See Also:


Attachments
[Patch] Proposed Fix (2.99 KB, patch)
2015-12-12 20:17 PST, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2015-12-12 20:12:59 PST
* SUMMARY
CodeMirrorTokenTrackingController handles symbols in class definitions incorrectly. CodeMirror throws an exception when hovering a class contractor or method definition. When walking tokens, we should be using `state.localState` instead of `state`, when available. Additionally, walking tokens while checking for object literal shorthand properties should halt if an open parenthesis is found.

* REDUCTION
<html>
<script>
class Foo {
    constructor() {}
    bar() {}
};
debugger;
</script>
</html>

* STEPS TO REPRODUCE
1. Inspect the above html
2. Reload the page, inspector pauses on `debugger` statement
3. Hover over the constructor or method in class Foo
  => Inspector crashes: External/CodeMirror/javascript.js:646:33: CONSOLE ERROR TypeError: state.tokenize is not a function. (In 'state.tokenize(stream, state)', 'state.tokenize' is undefined)
Comment 1 Matt Baker 2015-12-12 20:17:21 PST
Created attachment 267252 [details]
[Patch] Proposed Fix
Comment 2 WebKit Commit Bot 2015-12-12 21:10:08 PST
Comment on attachment 267252 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 267252

Committed r194013: <http://trac.webkit.org/changeset/194013>
Comment 3 WebKit Commit Bot 2015-12-12 21:10:12 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Joseph Pecoraro 2015-12-14 10:29:20 PST
Comment on attachment 267252 [details]
[Patch] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=267252&action=review

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:520
>              WebInspector.walkTokens(this._codeMirror, mode, position, function(tokenType, string) {
>                  if (tokenType)
>                      return false;
> +                if (string === "(")
> +                    return false;
>                  if (string === "," || string === "}") {
>                      shorthand = true;
>                      return false;

I thought we agreed on just checking for whitespace.

    return tokenType === null;
Comment 5 Radar WebKit Bug Importer 2015-12-14 10:29:45 PST
<rdar://problem/23883794>