Bug 164886 - [CSS Parser] Hook up InspectorStyleSheet to the new CSS parser.
Summary: [CSS Parser] Hook up InspectorStyleSheet to the new CSS parser.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-17 14:01 PST by Dave Hyatt
Modified: 2016-11-18 11:09 PST (History)
0 users

See Also:


Attachments
Patch (31.53 KB, patch)
2016-11-17 14:40 PST, Dave Hyatt
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2016-11-17 14:01:49 PST
[CSS Parser] Hook up InspectorStyleSheet to the new CSS parser.
Comment 1 Dave Hyatt 2016-11-17 14:40:00 PST
Created attachment 295087 [details]
Patch
Comment 2 Dean Jackson 2016-11-17 14:53:08 PST
Comment on attachment 295087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295087&action=review

> Source/WebCore/inspector/InspectorStyleSheet.cpp:199
> +    m_currentRuleDataStack.last()->ruleHeaderRange.end = listEndOffset;
> +    if (!m_currentRuleDataStack.last()->selectorRanges.isEmpty())
> +        m_currentRuleDataStack.last()->selectorRanges.last().end = listEndOffset;

Can m_currentRuleDataStack ever be empty?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:225
> +    // Skip the rule body opening brace.
> +    if (m_parsedText[offset] == '{')
> +        ++offset;

Should we skip any spaces as well?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:371
> +    bool parsedOk = propertyData.parsedOk || propertyData.name.startsWith("-moz-") || propertyData.name.startsWith("-o-") || propertyData.name.startsWith("-webkit-") || propertyData.name.startsWith("-ms-");

I don't understand this. All -moz- properties are ok? Shouldn't they be marked as not OK?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:378
> +    m_currentRuleDataStack.last()->styleSourceData->propertyData.append(CSSPropertySourceData(propertyData.name, propertyData.value, false, true, true, SourceRange(startOffset - topRuleBodyRange.start, endOffset - topRuleBodyRange.start)));

Is this the place where we set disabled to true?
Comment 3 Dave Hyatt 2016-11-18 11:09:30 PST
Fixed in r208886.