Bug 199130

Summary: Web Inspector: Styles: curly brace incorrectly added after completed value inside unclosed quote
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
hi: review+, hi: commit-queue-
Patch none

Nikita Vasilyev
Reported 2019-06-22 15:31:49 PDT
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.
Attachments
Patch (2.31 KB, patch)
2019-06-22 15:38 PDT, Nikita Vasilyev 		hi: review+
hi: commit-queue-
DetailsFormatted DiffDiff
Patch (2.29 KB, patch)
2019-07-06 18:27 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-06-22 15:33:08 PDT
<rdar://problem/52023534>
Nikita Vasilyev
Comment 2 2019-06-22 15:38:51 PDT
Created attachment 372690 [details] Patch
Devin Rousso
Comment 3 2019-07-05 12:20:31 PDT
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?
Nikita Vasilyev
Comment 4 2019-07-05 14:03:38 PDT
(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;
Nikita Vasilyev
Comment 5 2019-07-06 18:27:14 PDT
Created attachment 373589 [details] Patch
WebKit Commit Bot
Comment 6 2019-07-06 19:09:35 PDT
Comment on attachment 373589 [details] Patch Clearing flags on attachment: 373589 Committed r247196: <https://trac.webkit.org/changeset/247196>
WebKit Commit Bot
Comment 7 2019-07-06 19:09:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.