199130 2019-06-22 15:31:49 -0700 Web Inspector: Styles: curly brace incorrectly added after completed value inside unclosed quote 2019-07-06 19:09:36 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 nvasilyev nvasilyev commit-queue hi inspector-bugzilla-changes webkit-bug-importer oldest_to_newest 1547135 0 nvasilyev 2019-06-22 15:31:49 -0700 Steps: 1. Open https://webkit.org 2. Inspect <body> 3. Add `font-family: "Helv` CSS property to the Style Attribute rule. 4. Press Tab or Enter. Actual: font-family: "Helvetica}" Expected: font-family: "Helvetica" Notes: This is similar to https://bugs.webkit.org/show_bug.cgi?id=199090. Weirdly, for this case "}" gets added by WI.tokenizeCSSValue when it encounters unbalanced quotes. 1547137 1 webkit-bug-importer 2019-06-22 15:33:08 -0700 <rdar://problem/52023534> 1547138 2 372690 nvasilyev 2019-06-22 15:38:51 -0700 Created attachment 372690 Patch 1550468 3 372690 hi 2019-07-05 12:20:31 -0700 Comment on attachment 372690 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372690&action=review r=me, even though I have questions, the logic looks fine to me. Please address my questions before landing. > Source/WebInspectorUI/ChangeLog:9 > + `}` gets added by WI.tokenizeCSSValue (CodeMirror CSS tokenizer) when it encounters unbalanced quotes. It's worth mentioning that `WI.tokenizeCSSValue` is called by `_renderValue`, not in `spreadsheetTextFieldDidCommit`. That initially confused me. > Source/WebInspectorUI/ChangeLog:10 > + Fix unbalanced quotes by rendering the value from the model, not the view. Why does the model have the "correct" value, but the view doesn't? Every time you edit the view, it updates the model with the corresponding text, so the model and view should be in sync. Are you saying that they're not? If so, why? If not, what is actually changed/fixed by using the model? 1550512 4 nvasilyev 2019-07-05 14:03:38 -0700 (In reply to Devin Rousso from comment #3) > Comment on attachment 372690 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372690&action=review > > r=me, even though I have questions, the logic looks fine to me. Please > address my questions before landing. > > > Source/WebInspectorUI/ChangeLog:9 > > + `}` gets added by WI.tokenizeCSSValue (CodeMirror CSS tokenizer) when it encounters unbalanced quotes. > > It's worth mentioning that `WI.tokenizeCSSValue` is called by > `_renderValue`, not in `spreadsheetTextFieldDidCommit`. That initially > confused me. > > > Source/WebInspectorUI/ChangeLog:10 > > + Fix unbalanced quotes by rendering the value from the model, not the view. > > Why does the model have the "correct" value, but the view doesn't? Every > time you edit the view, it updates the model with the corresponding text, so > the model and view should be in sync. Are you saying that they're not? If > so, why? If not, what is actually changed/fixed by using the model? When I said the view, I really meant the DOM. After entering CSS value `"Helvetica` this is what happens: _handleValueChange() { this._property.rawValue = this._valueElement.textContent.trim(); } this._valueElement.textContent is "Helvetica. this._property.rawValue setter fixes unbalanced quotes, so the value becomes "Helvetica": set rawValue(value) { ... let suffix = WI.CSSCompletions.completeUnbalancedValue(value); if (suffix) value += suffix; 1550685 5 373589 nvasilyev 2019-07-06 18:27:14 -0700 Created attachment 373589 Patch 1550687 6 373589 commit-queue 2019-07-06 19:09:35 -0700 Comment on attachment 373589 Patch Clearing flags on attachment: 373589 Committed r247196: <https://trac.webkit.org/changeset/247196> 1550688 7 commit-queue 2019-07-06 19:09:36 -0700 All reviewed patches have been landed. Closing bug. 372690 2019-06-22 15:38:51 -0700 2019-07-06 18:27:14 -0700 Patch patch.txt text/plain 2363 nvasilyev ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDE3MTU1NWQyMjBjLi40MzA3ODc5NDBlYyAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTktMDYtMjIgIE5pa2l0 YSBWYXNpbHlldiAgPG52YXNpbHlldkBhcHBsZS5jb20+CisKKyAgICAgICAgV2ViIEluc3BlY3Rv cjogU3R5bGVzOiB1bmJhbGFuY2VkIHF1b3RlcyBhbmQgcGFyZW50aGVzaXMgYXJlbid0IGRpc3Bs YXllZCBhcyBwcm9wZXJ0eSBjbG9zZWQgYWZ0ZXIgZWRpdGluZyB2YWx1ZXMKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5OTA5MAorICAgICAgICA8cmRh cjovL3Byb2JsZW0vNTE5NjU0MzE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgYH1gIGdldHMgYWRkZWQgYnkgV0kudG9rZW5pemVDU1NWYWx1ZSAoQ29k ZU1pcnJvciBDU1MgdG9rZW5pemVyKSB3aGVuIGl0IGVuY291bnRlcnMgdW5iYWxhbmNlZCBxdW90 ZXMuCisgICAgICAgIEZpeCB1bmJhbGFuY2VkIHF1b3RlcyBieSByZW5kZXJpbmcgdGhlIHZhbHVl IGZyb20gdGhlIG1vZGVsLCBub3QgdGhlIHZpZXcuCisKKyAgICAgICAgKiBVc2VySW50ZXJmYWNl L1ZpZXdzL1NwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eS5qczoKKyAgICAgICAgKFdJLlNwcmVhZHNo ZWV0U3R5bGVQcm9wZXJ0eS5wcm90b3R5cGUuc3ByZWFkc2hlZXRUZXh0RmllbGREaWRDb21taXQp OgorICAgICAgICBGb3IgY29uc2lzdGVuY3ksIHJlbmRlciBwcm9wZXJ0eSBuYW1lIGZyb20gdGhl IG1vZGVsIGFzIHdlbGwuCisKIDIwMTktMDYtMjIgIE5pa2l0YSBWYXNpbHlldiAgPG52YXNpbHll dkBhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTihyMjQxOTgwKTogV2ViIEluc3BlY3Rv cjogU3R5bGVzOiBQcmVzc2luZyBUYWIvRW50ZXIgb24gbGFzdCBwcm9wZXJ0eSBubyBsb25nZXIg Zm9jdXNlcyBvbiBzZWxlY3RvciBvbiBuZXh0IHJ1bGUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJ bnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eS5q cyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NwcmVhZHNoZWV0 U3R5bGVQcm9wZXJ0eS5qcwppbmRleCA3ZjU5MzIzMWEyOC4uOTI2YmIwYTFmYjEgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NwcmVhZHNoZWV0 U3R5bGVQcm9wZXJ0eS5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFj ZS9WaWV3cy9TcHJlYWRzaGVldFN0eWxlUHJvcGVydHkuanMKQEAgLTM1OSwxNiArMzU5LDE0IEBA IFdJLlNwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eSA9IGNsYXNzIFNwcmVhZHNoZWV0U3R5bGVQcm9w ZXJ0eSBleHRlbmRzIFdJLk9iamVjdAogCiAgICAgc3ByZWFkc2hlZXRUZXh0RmllbGREaWRDb21t aXQodGV4dEZpZWxkLCB7ZGlyZWN0aW9ufSkKICAgICB7Ci0gICAgICAgIGxldCBwcm9wZXJ0eU5h bWUgPSB0aGlzLl9uYW1lVGV4dEZpZWxkLnZhbHVlLnRyaW0oKTsKLSAgICAgICAgbGV0IHByb3Bl cnR5VmFsdWUgPSB0aGlzLl92YWx1ZVRleHRGaWVsZC52YWx1ZS50cmltKCk7CiAgICAgICAgIGxl dCB3aWxsUmVtb3ZlUHJvcGVydHkgPSBmYWxzZTsKICAgICAgICAgbGV0IGlzRWRpdGluZ05hbWUg PSB0ZXh0RmllbGQgPT09IHRoaXMuX25hbWVUZXh0RmllbGQ7CiAKLSAgICAgICAgaWYgKCFwcm9w ZXJ0eU5hbWUgfHwgKCFwcm9wZXJ0eVZhbHVlICYmICFpc0VkaXRpbmdOYW1lICYmIGRpcmVjdGlv biA9PT0gImZvcndhcmQiKSkKKyAgICAgICAgaWYgKCF0aGlzLl9wcm9wZXJ0eS5uYW1lIHx8ICgh dGhpcy5fcHJvcGVydHkucmF3VmFsdWUgJiYgIWlzRWRpdGluZ05hbWUgJiYgZGlyZWN0aW9uID09 PSAiZm9yd2FyZCIpKQogICAgICAgICAgICAgd2lsbFJlbW92ZVByb3BlcnR5ID0gdHJ1ZTsKIAog ICAgICAgICBpZiAoIWlzRWRpdGluZ05hbWUgJiYgIXdpbGxSZW1vdmVQcm9wZXJ0eSkKLSAgICAg ICAgICAgIHRoaXMuX3JlbmRlclZhbHVlKHByb3BlcnR5VmFsdWUpOworICAgICAgICAgICAgdGhp cy5fcmVuZGVyVmFsdWUodGhpcy5fcHJvcGVydHkucmF3VmFsdWUpOwogCiAgICAgICAgIGlmIChk aXJlY3Rpb24gPT09ICJmb3J3YXJkIikgewogICAgICAgICAgICAgaWYgKGlzRWRpdGluZ05hbWUg JiYgIXdpbGxSZW1vdmVQcm9wZXJ0eSkgewo= 373589 2019-07-06 18:27:14 -0700 2019-07-06 19:09:35 -0700 Patch patch.txt text/plain 2346 nvasilyev ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGM0ZjE5YTc4ZGY3Li5iMzJlZTViMzRlYSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTktMDctMDYgIE5pa2l0 YSBWYXNpbHlldiAgPG52YXNpbHlldkBhcHBsZS5jb20+CisKKyAgICAgICAgV2ViIEluc3BlY3Rv cjogU3R5bGVzOiB1bmJhbGFuY2VkIHF1b3RlcyBhbmQgcGFyZW50aGVzaXMgYXJlbid0IGRpc3Bs YXllZCBhcyBwcm9wZXJ0eSBjbG9zZWQgYWZ0ZXIgZWRpdGluZyB2YWx1ZXMKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5OTA5MAorICAgICAgICA8cmRh cjovL3Byb2JsZW0vNTE5NjU0MzE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRGV2aW4gUm91c3Nv LgorCisgICAgICAgIGB9YCBnZXRzIGFkZGVkIGJ5IFdJLnRva2VuaXplQ1NTVmFsdWUgKGNhbGxl ZCBieSBTcHJlYWRzaGVldFN0eWxlUHJvcGVydHkucHJvdG90eXBlLl9yZW5kZXJWYWx1ZSkKKyAg ICAgICAgd2hlbiBpdCBlbmNvdW50ZXJzIHVuYmFsYW5jZWQgcXVvdGVzLiBGaXggdW5iYWxhbmNl ZCBxdW90ZXMgYnkgcmUtcmVuZGVyaW5nIHRoZSB2YWx1ZSBmcm9tIHRoZSBtb2RlbCwKKyAgICAg ICAgbm90IHRoZSBET00gY29udGVudC4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3Mv U3ByZWFkc2hlZXRTdHlsZVByb3BlcnR5LmpzOgorICAgICAgICAoV0kuU3ByZWFkc2hlZXRTdHls ZVByb3BlcnR5LnByb3RvdHlwZS5zcHJlYWRzaGVldFRleHRGaWVsZERpZENvbW1pdCk6CisgICAg ICAgIEZvciBjb25zaXN0ZW5jeSwgcmVuZGVyIHByb3BlcnR5IG5hbWUgZnJvbSB0aGUgbW9kZWwg YXMgd2VsbC4KKwogMjAxOS0wNy0wMiAgRGV2aW4gUm91c3NvICA8ZHJvdXNzb0BhcHBsZS5jb20+ CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogRWxlbWVudHM6IGFsbG93IG5vZGVzIHRvIGJlIGNv cGllZCBhbmQgcGFzdGVkCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlcklu dGVyZmFjZS9WaWV3cy9TcHJlYWRzaGVldFN0eWxlUHJvcGVydHkuanMgYi9Tb3VyY2UvV2ViSW5z cGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9TcHJlYWRzaGVldFN0eWxlUHJvcGVydHkuanMK aW5kZXggZGQ3YTBjNWY0ZTIuLjgzMGFkZmI5MzY2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5z cGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9TcHJlYWRzaGVldFN0eWxlUHJvcGVydHkuanMK KysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvU3ByZWFkc2hl ZXRTdHlsZVByb3BlcnR5LmpzCkBAIC0zNTksMTYgKzM1OSwxNCBAQCBXSS5TcHJlYWRzaGVldFN0 eWxlUHJvcGVydHkgPSBjbGFzcyBTcHJlYWRzaGVldFN0eWxlUHJvcGVydHkgZXh0ZW5kcyBXSS5P YmplY3QKIAogICAgIHNwcmVhZHNoZWV0VGV4dEZpZWxkRGlkQ29tbWl0KHRleHRGaWVsZCwge2Rp cmVjdGlvbn0pCiAgICAgewotICAgICAgICBsZXQgcHJvcGVydHlOYW1lID0gdGhpcy5fbmFtZVRl eHRGaWVsZC52YWx1ZS50cmltKCk7Ci0gICAgICAgIGxldCBwcm9wZXJ0eVZhbHVlID0gdGhpcy5f dmFsdWVUZXh0RmllbGQudmFsdWUudHJpbSgpOwogICAgICAgICBsZXQgd2lsbFJlbW92ZVByb3Bl cnR5ID0gZmFsc2U7CiAgICAgICAgIGxldCBpc0VkaXRpbmdOYW1lID0gdGV4dEZpZWxkID09PSB0 aGlzLl9uYW1lVGV4dEZpZWxkOwogCi0gICAgICAgIGlmICghcHJvcGVydHlOYW1lIHx8ICghcHJv cGVydHlWYWx1ZSAmJiAhaXNFZGl0aW5nTmFtZSAmJiBkaXJlY3Rpb24gPT09ICJmb3J3YXJkIikp CisgICAgICAgIGlmICghdGhpcy5fcHJvcGVydHkubmFtZSB8fCAoIXRoaXMuX3Byb3BlcnR5LnJh d1ZhbHVlICYmICFpc0VkaXRpbmdOYW1lICYmIGRpcmVjdGlvbiA9PT0gImZvcndhcmQiKSkKICAg ICAgICAgICAgIHdpbGxSZW1vdmVQcm9wZXJ0eSA9IHRydWU7CiAKICAgICAgICAgaWYgKCFpc0Vk aXRpbmdOYW1lICYmICF3aWxsUmVtb3ZlUHJvcGVydHkpCi0gICAgICAgICAgICB0aGlzLl9yZW5k ZXJWYWx1ZShwcm9wZXJ0eVZhbHVlKTsKKyAgICAgICAgICAgIHRoaXMuX3JlbmRlclZhbHVlKHRo aXMuX3Byb3BlcnR5LnJhd1ZhbHVlKTsKIAogICAgICAgICBpZiAoZGlyZWN0aW9uID09PSAiZm9y d2FyZCIpIHsKICAgICAgICAgICAgIGlmIChpc0VkaXRpbmdOYW1lICYmICF3aWxsUmVtb3ZlUHJv cGVydHkpIHsK