143539 2015-04-08 16:15:48 -0700 Web Inspector: Regression: Showing of color swatches no longer works in Details Sidebar 2015-04-08 18:03:19 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 tobi+webkit webkit-unassigned commit-queue graouts joepeck jonowells mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1083766 0 tobi+webkit 2015-04-08 16:15:48 -0700 I introduced a regression by trying to fix an ESLint issue. The ESLint report was correct but the fix was wrong. Commit that introduced the regression: http://svn.webkit.org/repository/webkit/trunk@182142 268f45cc-cd09-0410-ab3c-d52691b4dbfc https://github.com/WebKit/webkit/commit/b18ef637e495d9f64bca6e431d7ec71df8bc062d#diff-97910c4c4acd4a860a96c6592a3ab666L1013 1083767 1 webkit-bug-importer 2015-04-08 16:16:22 -0700 <rdar://problem/20474687> 1083768 2 250393 tobi+webkit 2015-04-08 16:24:02 -0700 Created attachment 250393 patch 1083769 3 250393 timothy 2015-04-08 16:34:03 -0700 Comment on attachment 250393 patch View in context: https://bugs.webkit.org/attachment.cgi?id=250393&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:396 > + this._codeMirror.setUniqueBookmark(codeMirrorTextMarker.find().from, swatchElement); I think codeMirrorTextMarker.find() can return null/undefined. Maybe this need a null check and early return? 1083770 4 250393 tobi+webkit 2015-04-08 16:52:47 -0700 Comment on attachment 250393 patch View in context: https://bugs.webkit.org/attachment.cgi?id=250393&action=review >> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:396 >> + this._codeMirror.setUniqueBookmark(codeMirrorTextMarker.find().from, swatchElement); > > I think codeMirrorTextMarker.find() can return null/undefined. Maybe this need a null check and early return? Nit: If that's the case I would rather create a new bug because I just re-enabled the code that was here before I removed it. Just without the variable declaration that initially triggered the ESLint report. I can investigate into the missing null check afterwards, if that's okay for you? 1083776 5 250393 timothy 2015-04-08 17:14:56 -0700 Comment on attachment 250393 patch View in context: https://bugs.webkit.org/attachment.cgi?id=250393&action=review >>> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:396 >>> + this._codeMirror.setUniqueBookmark(codeMirrorTextMarker.find().from, swatchElement); >> >> I think codeMirrorTextMarker.find() can return null/undefined. Maybe this need a null check and early return? > > Nit: If that's the case I would rather create a new bug because I just re-enabled the code that was here before I removed it. Just without the variable declaration that initially triggered the ESLint report. I can investigate into the missing null check afterwards, if that's okay for you? Fair enough. 1083797 6 250393 commit-queue 2015-04-08 18:03:11 -0700 Comment on attachment 250393 patch Clearing flags on attachment: 250393 Committed r182576: <http://trac.webkit.org/changeset/182576> 1083798 7 commit-queue 2015-04-08 18:03:19 -0700 All reviewed patches have been landed. Closing bug. 250393 2015-04-08 16:24:02 -0700 2015-04-08 18:03:11 -0700 patch bug-143539-20150409012311.patch text/plain 1940 tobi+webkit U3VidmVyc2lvbiBSZXZpc2lvbjogMTgyNTc0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCBm MWFjNTZkZmI5ZDFiMTI2ZDE4NzkzNjJkOTc2ZDg5Yjk2MzJlNmQ4Li4xZmEyZjEyZTQ0MzU3NmNk ZmY1MzczYzU0NGJkZmVmYTliOGQ1NWNlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNyBAQAorMjAxNS0wNC0wOCAgVG9iaWFzIFJlaXNzICA8dG9iaSt3ZWJraXRAYmFzZWNv ZGUuZGU+CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUmVncmVzc2lvbjogU2hvd2luZyBvZiBj b2xvciBzd2F0Y2hlcyBubyBsb25nZXIgd29ya3MgaW4gRGV0YWlscyBTaWRlYmFyCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDM1MzkKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGaXggYSByZWdyZXNzaW9u IHdoZXJlIGR1ZSB0byBhbiBFU0xpbnQgZXJyb3IgdGhlIHdob2xlIGxpbmUgaW5zdGVhZCBvZiBq dXN0CisgICAgICAgIHRoZSB2YXJpYWJsZSBkZWNsYXJhdGlvbiB3YXMgcmVtb3ZlZC4KKworICAg ICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvQ1NTU3R5bGVEZWNsYXJhdGlvblRleHRFZGl0b3Iu anM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuQ1NTU3R5bGVEZWNsYXJhdGlvblRleHRFZGl0b3Iu cHJvdG90eXBlLl9jcmVhdGVDb2xvclN3YXRjaGVzLnVwZGF0ZSk6CisgICAgICAgIChXZWJJbnNw ZWN0b3IuQ1NTU3R5bGVEZWNsYXJhdGlvblRleHRFZGl0b3IucHJvdG90eXBlLl9jcmVhdGVDb2xv clN3YXRjaGVzKToKKwogMjAxNS0wNC0wNyAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBw bGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IEVTNjogU2hvdyBTeW1ib2wgcHJvcGVy dGllcyBvbiBPYmplY3RzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlcklu dGVyZmFjZS9WaWV3cy9DU1NTdHlsZURlY2xhcmF0aW9uVGV4dEVkaXRvci5qcyBiL1NvdXJjZS9X ZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0NTU1N0eWxlRGVjbGFyYXRpb25UZXh0 RWRpdG9yLmpzCmluZGV4IDkzZGJlOWQzZWI3YzMzYTg0M2E0MmZlNDE3ODRkZjhmM2ZlNWZmZjEu LjcwM2M5NWJjMDU5Y2MyNzIxYTNiNTViMzA4NGNkOGM0ZmEwNjgyNTEgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0NTU1N0eWxlRGVjbGFyYXRp b25UZXh0RWRpdG9yLmpzCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L1ZpZXdzL0NTU1N0eWxlRGVjbGFyYXRpb25UZXh0RWRpdG9yLmpzCkBAIC0zOTMsNiArMzkzLDcg QEAgV2ViSW5zcGVjdG9yLkNTU1N0eWxlRGVjbGFyYXRpb25UZXh0RWRpdG9yID0gY2xhc3MgQ1NT U3R5bGVEZWNsYXJhdGlvblRleHRFZGl0b3IKICAgICAgICAgICAgICAgICBzd2F0Y2hFbGVtZW50 LmFwcGVuZENoaWxkKHN3YXRjaElubmVyRWxlbWVudCk7CiAKICAgICAgICAgICAgICAgICB2YXIg Y29kZU1pcnJvclRleHRNYXJrZXIgPSBtYXJrZXIuY29kZU1pcnJvclRleHRNYXJrZXI7CisgICAg ICAgICAgICAgICAgdGhpcy5fY29kZU1pcnJvci5zZXRVbmlxdWVCb29rbWFyayhjb2RlTWlycm9y VGV4dE1hcmtlci5maW5kKCkuZnJvbSwgc3dhdGNoRWxlbWVudCk7CiAKICAgICAgICAgICAgICAg ICBzd2F0Y2hJbm5lckVsZW1lbnQuX19jb2xvclRleHRNYXJrZXIgPSBjb2RlTWlycm9yVGV4dE1h cmtlcjsKICAgICAgICAgICAgICAgICBzd2F0Y2hJbm5lckVsZW1lbnQuX19jb2xvciA9IGNvbG9y Owo=