[Webkit-unassigned] [Bug 58244] Implement experimental CSS rule disabling/enabling feature

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=58244





--- Comment #1 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2011-04-11 10:27:25 PST ---
(From update of attachment 89022)
View in context: https://bugs.webkit.org/attachment.cgi?id=89022&action=review

Is this part of any spec? It might be worth to tie it into Annevk's CSSOM spec or similar effort.

Disabling a rule is clearly a super-unfriendly thing in WebKit -- forcing recalc of all styles for this one change seems like a waste of cycles. Perhaps we could start with understanding how this could be improved first?

Also, you're adding a bool to the CSSStyleRule. What's the memory impact of this change?

> Source/WebCore/css/CSSStyleRule.cpp:59
> +Document* CSSStyleRule::getDocument()

WebKit convention is to not have "get" prefix on getters. This should just be document()

> Source/WebCore/css/CSSStyleRule.cpp:77
> +    if (!doc) return;

Put return on its own line.

> Source/WebCore/css/CSSStyleRule.cpp:98
> +        result += " !disabled";

What's !disabled?

> Source/WebCore/css/CSSStyleRule.cpp:131
> +        if (doc) {
> +            doc->recalcStyle(Node::Force);
> +        }

No braces around one-liners.

> Source/WebCore/css/CSSStyleRule.cpp:142
> +        if (doc) {
> +            doc->recalcStyle(Node::Force);
> +        }

Ditto.

> Source/WebCore/css/CSSStyleRule.h:58
> +    CSSMutableStyleDeclaration* declaration() {

Func brace on new line.

> Source/WebCore/css/CSSStyleRule.h:64
> +        if (m_enabled)
> +            return m_style.get();
> +        if (!m_emptyStyle)
> +            m_emptyStyle = CSSMutableStyleDeclaration::create();
> +        return m_emptyStyle.get();
> +    }

Why is this in a header?

> Source/WebCore/css/CSSStyleRule.h:89
> +    static RefPtr<CSSMutableStyleDeclaration> m_emptyStyle;

Typically, we make these free-standing statics in the cpp file.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.