Bug 152218

Summary: Web Inspector: CodeMirrorTokenTrackingController handles symbols in class definitions incorrectly
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Description Flags
[Patch] Proposed Fix none

Description Matt Baker 2015-12-12 20:12:59 PST
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.

class Foo {
    constructor() {}
    bar() {}

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