Bug 178488

Summary:

Web Inspector: Styles Redesign: Newly added invalid property isn't immediately shown as invalid

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Nikita Vasilyev <nvasilyev>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, commit-queue, inspector-bugzilla-changes, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
[Animated GIF] Bug	none
Patch	bburg: review+, bburg: commit-queue-
[Animated GIF] With patch applied	none
Patch	none

Nikita Vasilyev

Reported 2017-10-18 15:22:30 PDT

Steps: 1. Add a property by "foo" 2. Press Enter 3. Type value name "bar" 4. Press Enter Expected: Property "foo: bar;" has a red strike-through. Actual: Property "foo: bar;" doesn't have a red strike-through. Updating the rule by toggling some other properties makes it show the red strike-through as expected.

Attachments
[Animated GIF] Bug (58.10 KB, image/gif) 2017-10-18 15:23 PDT, Nikita Vasilyev	no flags	Details
Patch (8.25 KB, patch) 2017-10-23 17:54 PDT, Nikita Vasilyev	bburg: review+ bburg: commit-queue-	Details Formatted Diff Diff
[Animated GIF] With patch applied (81.56 KB, image/gif) 2017-10-23 17:55 PDT, Nikita Vasilyev	no flags	Details
Patch (8.27 KB, patch) 2017-10-25 11:25 PDT, Nikita Vasilyev	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Nikita Vasilyev

Comment 1 2017-10-18 15:23:15 PDT

Created attachment 324175 [details] [Animated GIF] Bug

Radar WebKit Bug Importer

Comment 2 2017-10-18 15:23:44 PDT

<rdar://problem/35062131>

Nikita Vasilyev

Comment 3 2017-10-23 17:54:26 PDT

Created attachment 324620 [details] Patch

Nikita Vasilyev

Comment 4 2017-10-23 17:55:39 PDT

Created attachment 324621 [details] [Animated GIF] With patch applied HTML page for manual testing: http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/invalid.html

Blaze Burg

Comment 5 2017-10-25 09:06:42 PDT

Comment on attachment 324620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324620&action=review r=me > Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js:367 > + let index = insertAfterIndex + 1; Nit: propertyIndex > Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js:371 > + const dontFireEvents = true; Nit: suppressEvents or suppressEventDispatch > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:-128 > - this._element.classList.add(...classNames); I'd keep this as-is, unless there is some reason not to.

Nikita Vasilyev

Comment 6 2017-10-25 11:24:15 PDT

Comment on attachment 324620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324620&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:-128 >> - this._element.classList.add(...classNames); > > I'd keep this as-is, unless there is some reason not to. It used to be: this.element.className = ""; ... this._element.classList.add(...classNames); I replaced it with this._element.className = classNames.join(" "); to make it more concise.

Nikita Vasilyev

Comment 7 2017-10-25 11:25:51 PDT

Created attachment 324856 [details] Patch

WebKit Commit Bot

Comment 8 2017-10-25 12:02:32 PDT

Comment on attachment 324856 [details] Patch Clearing flags on attachment: 324856 Committed r223970: <https://trac.webkit.org/changeset/223970>

WebKit Commit Bot

Comment 9 2017-10-25 12:02:34 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.