Bug 197725 – Consider making CSSStyleSheet::rules() just an alias of CSSStyleSheet::cssRules().

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Emilio Cobos Álvarez (:emilio)]

Right now CSSStyleSheet.rules returns a StaticCSSRuleList. Blink got rid of this behavior, and I think WebKit could simplify their implementation too, so that it would match that and what I'm implementing in Firefox.

See https://github.com/w3c/csswg-drafts/pull/3900 / https://bugzilla.mozilla.org/show_bug.cgi?id=1545823

[Tyler Wilcock]

Created attachment 425139 [details]
Patch

[Darin Adler]

Comment on attachment 425139 [details]
Patch

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

> Source/WebCore/css/CSSStyleSheet.cpp:268
>  RefPtr<CSSRuleList> CSSStyleSheet::rules()
>  {
> -    if (!canAccessRules())
> -        return nullptr;
> -    // IE behavior.
> -    auto ruleList = StaticCSSRuleList::create();
> -    unsigned ruleCount = length();
> -    for (unsigned i = 0; i < ruleCount; ++i)
> -        ruleList->rules().append(item(i));
> -    return ruleList;
> +    return this->cssRules();
>  }

Four thoughts on this:

1) We should merge CSSStyleSheet::cssRulesForBindings with CSSStyleSheet::rulesForBindings, not just this lower layer.
2) We should use inlining, putting the forwarding in the header, if possible, and pay no runtime cost at all. So the function body of CSSStyleSheet::rulesForBindings should be in the header, not in the .cpp file.
3) We should delete CSSStyleSheet::rules entirely.
4) We can consider naming CSSStyleSheet::rulesForBindings to CSSStyleSheet::rules.

[Tyler Wilcock]

Created attachment 426281 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Tyler Wilcock]

Created attachment 426302 [details]
Patch

[Tyler Wilcock]

Thanks for the review, Darin!  I've done all those things and marked cq? now that EWS is green.

[Darin Adler]

Comment on attachment 426302 [details]
Patch

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

> Source/WebCore/css/CSSStyleSheet.h:67
> +    ExceptionOr<Ref<CSSRuleList>> rules() { return this->cssRulesForBindings(); }

The "this->" here is not needed.

[EWS]

Committed r276209 (236691@main): <https://commits.webkit.org/236691@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426302 [details].

[Radar WebKit Bug Importer]

<rdar://problem/77110091>