Bug 84251 – [shadow] needs style recalculation by changing applyAuthorStyles dynamically

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

[Takashi Sakamoto]

Currently applyAuthorStyles API doesn't cause style recalculation. Only when attaching elements (i.e. when creating renderer), applyAuthorStyles works. It should cause style recalculation.

[Takashi Sakamoto]

Created attachment 141647 [details]
Patch

[Hajime Morrita]

Comment on attachment 141647 [details]
Patch

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

In addition to reftests, It would be desired to check style value using widow.getComputedStyle()
before and after the second change. That allows you to ensure that the value is actually changed dynamically.

> Source/WebCore/dom/ShadowRoot.cpp:201
> +            owner()->needsStyleRecalc();

needsStyleRecalc() doesn't have any side effect.
It looks what you want is setNeedsReattachHostChildrenAndShadow().

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:94
> +    shadowRoot.applyAuthorStyles = true;

You need to force layout here (using offsetLeft or getComputedStyle like thing)

> LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:108
> +    shadowRoot.applyAuthorStyles = false;

Ditto.

[Takashi Sakamoto]

Created attachment 142162 [details]
Patch

[Takashi Sakamoto]

(In reply to comment #2)
> (From update of attachment 141647 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141647&action=review
> 
> In addition to reftests, It would be desired to check style value using widow.getComputedStyle()
> before and after the second change. That allows you to ensure that the value is actually changed dynamically.
> 
> > Source/WebCore/dom/ShadowRoot.cpp:201
> > +            owner()->needsStyleRecalc();
> 
> needsStyleRecalc() doesn't have any side effect.
> It looks what you want is setNeedsReattachHostChildrenAndShadow().

Done.

> > LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:94
> > +    shadowRoot.applyAuthorStyles = true;
> 
> You need to force layout here (using offsetLeft or getComputedStyle like thing)
> 
> > LayoutTests/fast/dom/shadow/shadow-root-applyAuthorStyles.html:108
> > +    shadowRoot.applyAuthorStyles = false;
> 
> Ditto.

I see. I added "div.offsetLeft".

[Takashi Sakamoto]

Created attachment 142682 [details]
Patch

[Takashi Sakamoto]

(In reply to comment #2)
> (From update of attachment 141647 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141647&action=review
> 
> In addition to reftests, It would be desired to check style value using widow.getComputedStyle()
> before and after the second change. That allows you to ensure that the value is actually changed dynamically.

Done.

[Hajime Morrita]

Comment on attachment 142682 [details]
Patch

Looks good.Thanks for the fix!

[WebKit Review Bot]

Comment on attachment 142682 [details]
Patch

Clearing flags on attachment: 142682

Committed r117716: <http://trac.webkit.org/changeset/117716>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.