148603 2015-08-28 20:07:56 -0700 Web Inspector: Debugger Popovers should work for object literal shorthand variables 2015-08-31 15:50:05 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg commit-queue graouts joepeck mattbaker nvasilyev saam timothy webkit-bug-importer oldest_to_newest 1121935 0 joepeck 2015-08-28 20:07:56 -0700 * 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! 1121936 1 webkit-bug-importer 2015-08-28 20:08:36 -0700 <rdar://problem/22487471> 1121937 2 260206 joepeck 2015-08-28 20:10:18 -0700 Created attachment 260206 [PATCH] Proposed Fix 1122199 3 260206 timothy 2015-08-31 10:12:18 -0700 Comment on attachment 260206 [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. 1122250 4 260206 joepeck 2015-08-31 12:23:35 -0700 Comment on attachment 260206 [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. 1122324 5 260206 commit-queue 2015-08-31 15:50:02 -0700 Comment on attachment 260206 [PATCH] Proposed Fix Clearing flags on attachment: 260206 Committed r189190: <http://trac.webkit.org/changeset/189190> 1122325 6 commit-queue 2015-08-31 15:50:05 -0700 All reviewed patches have been landed. Closing bug. 260206 2015-08-28 20:10:18 -0700 2015-08-31 15:50:02 -0700 [PATCH] Proposed Fix debugger-popover.patch text/plain 5117 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGI4ZTk5MWMuLmNhNGM2MDEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIxIEBACiAyMDE1LTA4LTI4ICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogRGVidWdn ZXIgUG9wb3ZlcnMgc2hvdWxkIHdvcmsgZm9yIG9iamVjdCBsaXRlcmFsIHNob3J0aGFuZCB2YXJp YWJsZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0 ODYwMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog VXNlckludGVyZmFjZS9Db250cm9sbGVycy9Db2RlTWlycm9yVG9rZW5UcmFja2luZ0NvbnRyb2xs ZXIuanM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuQ29kZU1pcnJvclRva2VuVHJhY2tpbmdDb250 cm9sbGVyLnByb3RvdHlwZS5fcHJvY2Vzc0phdmFTY3JpcHRFeHByZXNzaW9uKToKKyAgICAgICAg QWxsb3cgZGVidWdnZXIgcG9wb3ZlcnMgZm9yIG9iamVjdCBwcm9wZXJ0eSBzaG9ydGhhbmRzLCBi ZWNhdXNlIHRoZXkKKyAgICAgICAgYXJlIGFjdHVhbGx5IHZhcmlhYmxlcyB0aGF0IGhhdmUgdmFs dWVzLiBQcmV2aW91c2x5IHdlIGlnbm9yZWQgcHJvcGVydHkKKyAgICAgICAgbmFtZSBsaXRlcmFs cywgYmVjYXVzZSB0aGV5IHdlcmUganVzdCBwcm9wZXJ0eSBuYW1lcyB1bnRpbCBFUzYuCisKKyAg ICAgICAgKiBVc2VySW50ZXJmYWNlL1ZpZXdzL0NvZGVNaXJyb3JBZGRpdGlvbnMuanM6CisgICAg ICAgIEFkZCBhIGhlbHBlciBmb3Igd2Fsa2luZyB0aHJvdWdoIHRva2VuIHR5cGVzIGZyb20gYSBz dGFydCBwb3NpdGlvbi4gSXQgbWF5IGJlIHVzZWZ1bCBlbHNld2hlcmUuCisKKzIwMTUtMDgtMjgg IEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKwogICAgICAgICBXZWIgSW5z cGVjdG9yOiBSZXNvdXJjZS5wcm90b3R5cGUuYXNzb2NpYXRlV2l0aFNjcmlwdCBhc3NlcnRpb25z IGZpcmluZwogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTQ4NjAxCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L0NvbnRyb2xsZXJzL0NvZGVNaXJyb3JUb2tlblRyYWNraW5nQ29udHJvbGxlci5qcyBiL1NvdXJj ZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL0NvbnRyb2xsZXJzL0NvZGVNaXJyb3JUb2tl blRyYWNraW5nQ29udHJvbGxlci5qcwppbmRleCA3OGQ3NmNlLi4xNDQ3YTcyIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9Db250cm9sbGVycy9Db2RlTWly cm9yVG9rZW5UcmFja2luZ0NvbnRyb2xsZXIuanMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJ L1VzZXJJbnRlcmZhY2UvQ29udHJvbGxlcnMvQ29kZU1pcnJvclRva2VuVHJhY2tpbmdDb250cm9s bGVyLmpzCkBAIC00NTIsMTAgKzQ1MiwyNiBAQCBXZWJJbnNwZWN0b3IuQ29kZU1pcnJvclRva2Vu VHJhY2tpbmdDb250cm9sbGVyID0gY2xhc3MgQ29kZU1pcnJvclRva2VuVHJhY2tpbmdDbwogICAg ICAgICBpZiAoIWlzUHJvcGVydHkgJiYgIWlzS2V5d29yZCAmJiB0eXBlLmluZGV4T2YoInZhcmlh YmxlIikgPT09IC0xICYmIHR5cGUuaW5kZXhPZigiZGVmIikgPT09IC0xKQogICAgICAgICAgICAg cmV0dXJuIG51bGw7CiAKLSAgICAgICAgLy8gTm90IG9iamVjdCBsaXRlcmFsIHByb3BlcnRpZXMu Ci0gICAgICAgIHZhciBzdGF0ZSA9IHRoaXMuX2hvdmVyZWRUb2tlbkluZm8uaW5uZXJNb2RlLnN0 YXRlOwotICAgICAgICBpZiAoaXNQcm9wZXJ0eSAmJiBzdGF0ZS5sZXhpY2FsICYmIHN0YXRlLmxl eGljYWwudHlwZSA9PT0gIn0iKQotICAgICAgICAgICAgcmV0dXJuIG51bGw7CisgICAgICAgIC8v IE5vdCBvYmplY3QgbGl0ZXJhbCBwcm9wZXJ0eSBuYW1lcywgYnV0IHllcyBpZiBhbiBvYmplY3Qg bGl0ZXJhbCBzaG9ydGhhbmQgcHJvcGVydHksIHdoaWNoIGlzIGEgdmFyaWFibGUuCisgICAgICAg IGxldCBzdGF0ZSA9IHRoaXMuX2hvdmVyZWRUb2tlbkluZm8uaW5uZXJNb2RlLnN0YXRlOworICAg ICAgICBpZiAoaXNQcm9wZXJ0eSAmJiBzdGF0ZS5sZXhpY2FsICYmIHN0YXRlLmxleGljYWwudHlw ZSA9PT0gIn0iKSB7CisgICAgICAgICAgICAvLyBQZWVrIGFoZWFkIHRvIHNlZSBpZiB0aGUgbmV4 dCB0b2tlbiBpcyAifSIgb3IgIiwiLiBJZiBpdCBpcywgd2UgYXJlIGEgc2hvcnRoYW5kIGFuZCB0 aGVyZWZvcmUgYSB2YXJpYWJsZS4KKyAgICAgICAgICAgIGxldCBzaG9ydGhhbmQgPSBmYWxzZTsK KyAgICAgICAgICAgIGxldCBtb2RlID0gdGhpcy5faG92ZXJlZFRva2VuSW5mby5pbm5lck1vZGUu bW9kZTsKKyAgICAgICAgICAgIGxldCBwb3NpdGlvbiA9ICB7bGluZTogdGhpcy5faG92ZXJlZFRv a2VuSW5mby5wb3NpdGlvbi5saW5lLCBjaDogdGhpcy5faG92ZXJlZFRva2VuSW5mby50b2tlbi5l bmR9OworICAgICAgICAgICAgV2ViSW5zcGVjdG9yLndhbGtUb2tlbnModGhpcy5fY29kZU1pcnJv ciwgbW9kZSwgcG9zaXRpb24sIGZ1bmN0aW9uKHRva2VuVHlwZSwgc3RyaW5nKSB7CisgICAgICAg ICAgICAgICAgaWYgKHRva2VuVHlwZSkKKyAgICAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNl OworICAgICAgICAgICAgICAgIGlmIChzdHJpbmcgPT09ICIsIiB8fCBzdHJpbmcgPT09ICJ9Iikg eworICAgICAgICAgICAgICAgICAgICBzaG9ydGhhbmQgPSB0cnVlOworICAgICAgICAgICAgICAg ICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgICAgICAgICAgfQorICAgICAgICAgICAgICAgIHJl dHVybiB0cnVlOworICAgICAgICAgICAgfSk7CisKKyAgICAgICAgICAgIGlmICghc2hvcnRoYW5k KQorICAgICAgICAgICAgICAgIHJldHVybiBudWxsOworICAgICAgICB9CiAKICAgICAgICAgLy8g T25seSB0aGUgInRoaXMiIGtleXdvcmQuCiAgICAgICAgIGlmIChpc0tleXdvcmQgJiYgdGhpcy5f aG92ZXJlZFRva2VuSW5mby50b2tlbi5zdHJpbmcgIT09ICJ0aGlzIikKQEAgLTQ2NSw3ICs0ODEs NyBAQCBXZWJJbnNwZWN0b3IuQ29kZU1pcnJvclRva2VuVHJhY2tpbmdDb250cm9sbGVyID0gY2xh c3MgQ29kZU1pcnJvclRva2VuVHJhY2tpbmdDbwogICAgICAgICB2YXIgZXhwcmVzc2lvbiA9IHRo aXMuX2hvdmVyZWRUb2tlbkluZm8udG9rZW4uc3RyaW5nOwogICAgICAgICB2YXIgZXhwcmVzc2lv blN0YXJ0UG9zaXRpb24gPSB7bGluZTogdGhpcy5faG92ZXJlZFRva2VuSW5mby5wb3NpdGlvbi5s aW5lLCBjaDogdGhpcy5faG92ZXJlZFRva2VuSW5mby50b2tlbi5zdGFydH07CiAgICAgICAgIHdo aWxlICh0cnVlKSB7Ci0gICAgICAgICAgICB2YXIgdG9rZW4gPSB0aGlzLl9jb2RlTWlycm9yLmdl dFRva2VuQXQoZXhwcmVzc2lvblN0YXJ0UG9zaXRpb24pOyAgICAgICAgICAgIAorICAgICAgICAg ICAgdmFyIHRva2VuID0gdGhpcy5fY29kZU1pcnJvci5nZXRUb2tlbkF0KGV4cHJlc3Npb25TdGFy dFBvc2l0aW9uKTsKICAgICAgICAgICAgIGlmICghdG9rZW4pCiAgICAgICAgICAgICAgICAgYnJl YWs7CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1Zp ZXdzL0NvZGVNaXJyb3JBZGRpdGlvbnMuanMgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlcklu dGVyZmFjZS9WaWV3cy9Db2RlTWlycm9yQWRkaXRpb25zLmpzCmluZGV4IGM5MDE3OWIuLjUyM2Zi ZTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdz L0NvZGVNaXJyb3JBZGRpdGlvbnMuanMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJ bnRlcmZhY2UvVmlld3MvQ29kZU1pcnJvckFkZGl0aW9ucy5qcwpAQCAtNTUyLDMgKzU1MiwzMCBA QCBXZWJJbnNwZWN0b3IuY29tcGFyZUNvZGVNaXJyb3JQb3NpdGlvbnMgPSBmdW5jdGlvbihhLCBi KQogICAgIHZhciBiQ29sdW1uID0gImNoIiBpbiBiID8gYi5jaCA6IE51bWJlci5NQVhfVkFMVUU7 CiAgICAgcmV0dXJuIGFDb2x1bW4gLSBiQ29sdW1uOwogfTsKKworV2ViSW5zcGVjdG9yLndhbGtU b2tlbnMgPSBmdW5jdGlvbihjbSwgbW9kZSwgaW5pdGlhbFBvc2l0aW9uLCBjYWxsYmFjaykKK3sK KyAgICBsZXQgc3RhdGUgPSBDb2RlTWlycm9yLmNvcHlTdGF0ZShtb2RlLCBjbS5nZXRUb2tlbkF0 KGluaXRpYWxQb3NpdGlvbikuc3RhdGUpOworICAgIGxldCBsaW5lQ291bnQgPSBjbS5saW5lQ291 bnQoKTsKKworICAgIGxldCBhYm9ydCA9IGZhbHNlOworICAgIGZvciAobGluZU51bWJlciA9IGlu aXRpYWxQb3NpdGlvbi5saW5lOyAhYWJvcnQgJiYgbGluZU51bWJlciA8IGxpbmVDb3VudDsgKyts aW5lTnVtYmVyKSB7CisgICAgICAgIGxldCBsaW5lID0gY20uZ2V0TGluZShsaW5lTnVtYmVyKTsK KyAgICAgICAgbGV0IHN0cmVhbSA9IG5ldyBDb2RlTWlycm9yLlN0cmluZ1N0cmVhbShsaW5lKTsK KyAgICAgICAgaWYgKGxpbmVOdW1iZXIgPT09IGluaXRpYWxQb3NpdGlvbi5saW5lKQorICAgICAg ICAgICAgc3RyZWFtLnN0YXJ0ID0gc3RyZWFtLnBvcyA9IGluaXRpYWxQb3NpdGlvbi5jaDsKKwor ICAgICAgICB3aGlsZSAoIXN0cmVhbS5lb2woKSkgeworICAgICAgICAgICAgbGV0IGlubmVyTW9k ZSA9IENvZGVNaXJyb3IuaW5uZXJNb2RlKG1vZGUsIHN0YXRlKTsKKyAgICAgICAgICAgIGxldCB0 b2tlblR5cGUgPSBtb2RlLnRva2VuKHN0cmVhbSwgc3RhdGUpOworICAgICAgICAgICAgaWYgKCFj YWxsYmFjayh0b2tlblR5cGUsIHN0cmVhbS5jdXJyZW50KCkpKSB7CisgICAgICAgICAgICAgICAg YWJvcnQgPSB0cnVlOworICAgICAgICAgICAgICAgIGJyZWFrOworICAgICAgICAgICAgfQorICAg ICAgICAgICAgc3RyZWFtLnN0YXJ0ID0gc3RyZWFtLnBvczsKKyAgICAgICAgfQorICAgIH0KKwor ICAgIGlmICghYWJvcnQpCisgICAgICAgIGNhbGxiYWNrKG51bGwpOworfQo=