[Webkit-unassigned] [Bug 78472] ShadowRoot needs applyAuthorStyles

Sat Apr 21 18:26:42 PDT 2012

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

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #137719|review?                     |review-
               Flag|                            |

--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org>  2012-04-21 18:26:41 PST ---
(From update of attachment 137719)
View in context: https://bugs.webkit.org/attachment.cgi?id=137719&action=review

> Source/WebCore/ChangeLog:12
> +        seems to work in the same way as applyAuthorStyles, just renamed all applyAuthorSheets and
> +        added applyAuthorStyles to ShadowRoot.idl.

Nit: renamed all applyAuthorSheets to applyAuthorStyles, and added applyAuthorStyles to ShadowRoot.idl.

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles-expected.html:4
> +<head>
> +</head>

We don't need head for this, do we?

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles-expected.html:7
> +<div id="apply-author-style"><span style="margin:2px; border:solid"></span></div>
> +<div id="no-apply-author-style"><span></span></div>

Can we add more test cases like font-size, color, background-color, etc...?
Also, you should definitely test cases where the author style is overridden by inline style declaration or
cases where it overrides UA rules or when it's overridden by important UA rules.
Furthermore, you should test a case where you change the value of applyAuthorStyles dynamically and
verify that the change takes place dynamically as expected.

r- due to lack of adequate tests.

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:27
> +    var sr = new WebKitShadowRoot(div);

Please don't use abbreviations like sr. Also, does this constructor work on ports that don't enable shadow root support yet?

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:31
> +    var span = document.createElement('span');
> +    sr.appendChild(span);

Since you're not using span elsewhere, you can just do:
sr.appendChild(document.createElement('span'));

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