Bug 99826 - Web Inspector: Crash when adding a keyframes rule in the Styles pane
Summary: Web Inspector: Crash when adding a keyframes rule in the Styles pane
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-19 03:15 PDT by Alexander Pavlov (apavlov)
Modified: 2012-10-23 08:57 PDT (History)
9 users (show)

See Also:

Attachments
Patch (5.35 KB, patch)
2012-10-19 05:43 PDT, Alexander Pavlov (apavlov) 		no flags Details | Formatted Diff | Diff
Patch (9.08 KB, patch)
2012-10-19 06:31 PDT, Alexander Pavlov (apavlov) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2012-10-19 03:15:20 PDT 
In the Styles pane, add a new CSS rule with the selector "@-webkit-keyframes shake" (unquoted). Bam!

Upstreaming http://code.google.com/p/chromium/issues/detail?id=156516
Comment 1 Alexander Pavlov (apavlov) 2012-10-19 05:43:10 PDT 
Created attachment 169605 [details]
Patch
Comment 2 Pavel Feldman 2012-10-19 05:52:28 PDT 
Comment on attachment 169605 [details]
Patch

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

> Source/WebCore/inspector/InspectorStyleSheet.cpp:816
> +    CSSParser p(m_pageStyleSheet->ownerDocument() ? CSSParserContext(m_pageStyleSheet->ownerDocument()) : strictCSSParserContext());

We use different parser constructors in this class. Why? Could we introduce factory methods with descriptive names?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:821
> +        return 0;

Do you want to extract this into a separate method?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:845
> +        m_pageStyleSheet->deleteRule(lastRuleIndex, ec);

You should assign exception code there.

> LayoutTests/inspector/styles/styles-add-new-rule.html:59
> +    function step6()

Please add test per case.
Comment 3 Alexander Pavlov (apavlov) 2012-10-19 06:31:13 PDT 
Created attachment 169610 [details]
Patch
Comment 4 WebKit Review Bot 2012-10-23 08:57:43 PDT 
Comment on attachment 169610 [details]
Patch

Clearing flags on attachment: 169610

Committed r132229: <http://trac.webkit.org/changeset/132229>
Comment 5 WebKit Review Bot 2012-10-23 08:57:47 PDT 
All reviewed patches have been landed.  Closing bug.