Bug 148603 – Web Inspector: Debugger Popovers should work for object literal shorthand variables

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Debugger Popovers should work for object literal shorthand variables.

* TEST
>  1. var lineNumber = 10;
>  2. var columnNumber = 20;
>  3. var o0 = { lineNumber: lineNumber, columnNumber: columnNumber };
>  4. var o1 = { lineNumber, columnNumber };
>  5. var o2 = { lineNumber , columnNumber };
>  6. var o2 = {
>  7.     lineNumber
>  8.     ,
>  9.     columnNumber
> 10. };
> 11. console.log(o1, o2);

* STEPS TO REPRODUCE
1. Pause on line 11 in the test case.
2. Hover lineNumber/columnNumber anywhere it is treated as a variable
  => Expected to see their values, but got nothing on lines 4-10

* NOTES
- We should really eventually move to having an Esprima AST where it makes sense, like here. The AST Node for this position would just give us the info we want!

[Radar WebKit Bug Importer]

<rdar://problem/22487471>

[Joseph Pecoraro]

Created attachment 260206 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

Comment on attachment 260206 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:558
> +    let state = CodeMirror.copyState(mode, cm.getTokenAt(initialPosition).state);

Why does it need to be a copy? Because you use StringStream?

Why can't we walk the existing tokens that are already parsed by the CodeMirror instance? CodeMirrorCompletionController and CodeMirrorTokenTrackingController loop using getTokenAt.

[Joseph Pecoraro]

Comment on attachment 260206 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:558
>> +    let state = CodeMirror.copyState(mode, cm.getTokenAt(initialPosition).state);
> 
> Why does it need to be a copy? Because you use StringStream?
> 
> Why can't we walk the existing tokens that are already parsed by the CodeMirror instance? CodeMirrorCompletionController and CodeMirrorTokenTrackingController loop using getTokenAt.

I suppose it doesn't need to be a copy, but as a generic algorithm I wanted it to be safe.

When I tried getTokenAt(...) it was skipping over whitespace and just giving me the next token. In this case, it wasn't giving me a token for the comma. mode.token() gives me the token type but more importantly each substring, so I could detect the "," or "}" needed here.

[WebKit Commit Bot]

Comment on attachment 260206 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 260206

Committed r189190: <http://trac.webkit.org/changeset/189190>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.