[Webkit-unassigned] [Bug 87805] [Shadow DOM] <style> inside Shadow subtree should be scoped inside the subtree.

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2012-06-07 14:50:13 PST ---
(In reply to comment #6)
> Thank you for reviewing.
> 
> I will recreate a patch. However I would like to ask one question. We cannot rely on inDocument() to determine whether style elements are registered or not?

I am not sure what you mean by "style elements are registered" here. If you mean "registered with a scoping node", then obviously no. when inDocument() == false, the node should not be registered with any scoping node, but when inDocument() == true, it may or may not be registered with a scoping node.

If you mean something else, you may have to explain it a bit more :)

> If using scoped() and m_isRegisteredWithScopingNode in the same way (=the original code's way), I think, we cannot determine where the style was registered when scoped attribute is changed to be true. I tested this by the following example:
> 
>    <style id="mystyle" scoped>div { color: red };</style>
> 
>    var style = document.getElementById("mystyle");
>    style.setAttribute("scoped", "a"); // parseAttribute is invoked with !value.isNull
>    style.setAttribute("scoped", "b"); // parseAttribute is invoked with !value.isNull
>    style.setAttribute("scoped", "c"); // parseAttribute is invoked with !value.isNull
>    style.setAttribute("scoped", "d"); // parseAttribute is invoked with !value.isNull

I don't understand what you're testing here. Can you explain a bit more?

> 
> During HTMLStyleElement::parseAttribute, we can only know whether currently scoped or not and whether currently in shadow tree or not. So we cannot determine:
> 
>   (a) [old] in shadow DOM subtree and not scoped --> [new] in shadow DOM subtree and scoped
>   (b) [old] in shadow DOM subtree and scoped --> [new] in shadow DOM subtree and scoped
> 
> In the case (b), we don't need to update. But in the case (a), we have to remove the style from shadow root. However in the both case, m_isRegisteredWithScopingNode is true, scoped() is true and isInShadowTree() is true.

So, what you need is a knowledge of how the scoping came about: whether by virtue of just being in shadow DOM subtree, or with a scoped attribute. I think I understand.

> 
> This is the reason why I removed m_isRegisteredWithScopingNode and I added the layout test, setAttribute("scoped", "scoped") for already scoped style.
> 
> I think, there are several ways to fix the above issue:
> (1) modifying scoped() to see m_scoped, i.e. bool scoped() { return m_scoped; }

This is wrong. scoped() should always reflect the attribute value.

> (2) modifying the type of m_isRegisteredWithScopingNode from bool to enum { NotRegistered, RegisteredAsScoped, RegisteredInShadowTree }.

This seems good.

> (3) adding a new boolean flag to find where a style element was registered, e.g. m_isRegisteredInShadowRoot or something.

Here, you will always have one invalid combination, which seems like a bad thing.

Thank you for explaining this! I am glad you're tackling such complex problem and making progress. Keep going! :)

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