Bug 170281

Summary: Web Inspector: Styles sidebar warning icon appears inside property value text
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Video] misplaced warning icon
none
Patch
none
[Image] w/ patch applied none

Matt Baker
Reported 2017-03-30 04:25:13 PDT
Created attachment 305854 [details] [Video] misplaced warning icon Summary: Styles sidebar warning icon appears inside property value text Steps to Reproduce: 1. Inspect test case 2. Elements tab > expand <body> 3. Open Styles sidebar 4. Select first <p> child of <body> => Note warning icon for invalid CSS property value 5. Use arrow down/up keys to cycle through sibling <p> nodes => Warning icon tramples through the property value Test Case: <body> <p style="color:3.14"></p> <p style="color: 3.14"></p> <p style="color: 3.14"></p> <p style="color: 3.14"></p> <p style="color: 3.14"></p> <p style="color: 3.14"></p> </body>
Attachments
[Video] misplaced warning icon (306.87 KB, video/mp4)
2017-03-30 04:25 PDT, Matt Baker 		no flags
Details
Patch (3.22 KB, patch)
2017-03-30 05:10 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
[Image] w/ patch applied (100.78 KB, image/png)
2017-03-30 05:14 PDT, Matt Baker 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Matt Baker
Comment 1 2017-03-30 04:48:46 PDT
Regressed in https://bugs.webkit.org/show_bug.cgi?id=146617.
Matt Baker
Comment 2 2017-03-30 05:05:38 PDT
The position of the warning icon accounted for whitespace in the text while editing, but after CodeMirror updates and applies formatting the icon (and strikethrough range) is incorrect. Placing an icon near the value is error prone (it's potentially a moving target during editing), and doesn't look very good. Proposal: I suggest we not move the warning icon based on the location of the error (property name vs value). It's error prone, and visually jarring. The strikethrough indicates the part of the property with the error.
Matt Baker
Comment 3 2017-03-30 05:10:04 PDT
Created attachment 305855 [details] Patch
Matt Baker
Comment 4 2017-03-30 05:14:04 PDT
Created attachment 305856 [details] [Image] w/ patch applied
Devin Rousso
Comment 5 2017-03-30 10:03:23 PDT
(In reply to Matt Baker from comment #2) > The position of the warning icon accounted for whitespace in the text while > editing, but after CodeMirror updates and applies formatting the icon (and > strikethrough range) is incorrect. > > Placing an icon near the value is error prone (it's potentially a moving > target during editing), and doesn't look very good. > > Proposal: > I suggest we not move the warning icon based on the location of the error > (property name vs value). It's error prone, and visually jarring. The > strikethrough indicates the part of the property with the error. I personally would rather have the warning icon match the position of the strikethrough. I think a simpler solution to this would be to place the warning icon immediately after the semicolon and before all spacing, since we know that the spaces will be collapsed anyways. Having the warning icon always be at the beginning of the text implies to me an issue with the property as a whole (such as the name not existing), not just it's value.
WebKit Commit Bot
Comment 6 2017-03-30 13:17:18 PDT
Comment on attachment 305855 [details] Patch Clearing flags on attachment: 305855 Committed r214617: <http://trac.webkit.org/changeset/214617>
WebKit Commit Bot
Comment 7 2017-03-30 13:17:22 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.