Bug 180687 - REGRESSION (r225569): Web Inspector: Commented out properties aren't properly highlighted
Summary: REGRESSION (r225569): Web Inspector: Commented out properties aren't properly...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-11 17:39 PST by Nikita Vasilyev
Modified: 2017-12-18 13:50 PST (History)
6 users (show)

See Also:


Attachments
[Image] Bug (15.75 KB, image/png)
2017-12-11 17:39 PST, Nikita Vasilyev
no flags Details
Patch (2.42 KB, patch)
2017-12-14 22:02 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-elcapitan (2.89 MB, application/zip)
2017-12-15 01:37 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2017-12-11 17:39:21 PST
Created attachment 329064 [details]
[Image] Bug

Everything between of /* */ should be green.
Comment 1 Devin Rousso 2017-12-14 22:02:48 PST
Created attachment 329461 [details]
Patch
Comment 2 Nikita Vasilyev 2017-12-14 23:59:36 PST
I addressed this in the patch for Bug 180676 - Web Inspector: Styles Redesign: tabbing on commented out property throws exception, which still needs to be reviewed. I'm sorry I didn't mention it in this bug.
Comment 3 Devin Rousso 2017-12-15 00:16:04 PST
(In reply to Nikita Vasilyev from comment #2)
> I addressed this in the patch for Bug 180676 - Web Inspector: Styles
> Redesign: tabbing on commented out property throws exception, which still
> needs to be reviewed. I'm sorry I didn't mention it in this bug.
I haven't had a chance to test <https://webkit.org/b/180676> fully yet, but from what I can see I think the approach taken here is safer/cleaner.  I would either recommend landing it separately (since we have two bugzilla anyways) or changing it to match what is here.
Comment 4 Nikita Vasilyev 2017-12-15 00:23:53 PST
Comment on attachment 329461 [details]
Patch

I'm fine with lending these two separately.

The patch looks good. `.property:not(.disabled)` is consistent with what we already use for .token-link, .token-string, and .token-comment. I'd r+, but I'm not a reviewer.
Comment 5 EWS Watchlist 2017-12-15 01:37:51 PST Comment hidden (obsolete)
Comment 6 EWS Watchlist 2017-12-15 01:37:55 PST Comment hidden (obsolete)
Comment 7 Devin Rousso 2017-12-18 13:28:37 PST
Comment on attachment 329461 [details]
Patch

The test failure is unrelated (caused by <https://webkit.org/b/180770>).
Comment 8 WebKit Commit Bot 2017-12-18 13:49:10 PST
Comment on attachment 329461 [details]
Patch

Clearing flags on attachment: 329461

Committed r226075: <https://trac.webkit.org/changeset/226075>
Comment 9 WebKit Commit Bot 2017-12-18 13:49:12 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2017-12-18 13:50:36 PST
<rdar://problem/36114984>