| Summary: | Web Inspector: Debugger Popovers should work for object literal shorthand variables | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
| Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, saam, timothy, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
Created attachment 260206 [details]
[PATCH] Proposed Fix
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. 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. Comment on attachment 260206 [details] [PATCH] Proposed Fix Clearing flags on attachment: 260206 Committed r189190: <http://trac.webkit.org/changeset/189190> All reviewed patches have been landed. Closing bug. |
* 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!