[webkit-reviews] review granted: [Bug 199130] Web Inspector: Styles: curly brace incorrectly added after completed value inside unclosed quote : [Attachment 372690] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 199130: Web Inspector: Styles: curly brace incorrectly added after
completed value inside unclosed quote
https://bugs.webkit.org/show_bug.cgi?id=199130

Attachment 372690: Patch

https://bugs.webkit.org/attachment.cgi?id=372690&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 372690
  --> https://bugs.webkit.org/attachment.cgi?id=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?