Bug 99552 – The order of resolving distribution in tree composition is wrong

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

[Shinya Kawanaka]

When we have the following shadow dom, we should resolve distribution with the SR3 -> SR2 -> SR1 -> SR4 order.

host -- SR1 -- SR2 -- SR3
                |
                |- A ------ SR4
                   |         |
                   |- B      | -- content
                   |
                   |- content

[Shinya Kawanaka]

Hmm... It seems the current algorithm works correctly.

[Shinya Kawanaka]

Hmm... I became not sure it's working correctly. At least, we have to have a test.

[Shinya Kawanaka]

According to the current spec (W3C Editor's Draft 23 October 2012), we have to resolve <content> first.

4. Repeat while TREE exists:
  1. Let POINT be the first encountered active shadow insertion point in TREE, in tree order
  2. Run the distribution algorithm, supplying POOL and TREE as input
  3. If POINT exists:
    ....

https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html

We're resolving both <shadow> and <content> in tree order.

[Shinya Kawanaka]

Created attachment 170055 [details]
Repro

We have to show like the following:
  C
  B
  A

However, the first <shadow> selects all div now.

[Shinya Kawanaka]

Now I have a patch for this, but this causes 30+ errors in fast/dom/shadow/.
It seems the tests are wrong...!!

[Shinya Kawanaka]

Created attachment 170065 [details]
WIP

[Shinya Kawanaka]

(In reply to comment #5)
> Now I have a patch for this, but this causes 30+ errors in fast/dom/shadow/.
> It seems the tests are wrong...!!

Oops, not all the tests are wrong. My patch still has some bug maybe.

[Shinya Kawanaka]

Ahhhhhhh... I have a very stupid mistake in the WIP patch :-/

[Shinya Kawanaka]

Created attachment 170070 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 170070 [details]
Patch

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

> Source/WebCore/html/shadow/ContentDistributor.cpp:107
> +        if (ElementShadow* shadow = firstActiveShadowInsertionPointInOldestShadowRoot->parentNode()->isElementNode() ? toElement(firstActiveShadowInsertionPointInOldestShadowRoot->parentNode())->shadow() : 0)

I think you can turn this into a helper function -- you use it twice in this code, and it might be good to have a nice name explaining the purpose.

[WebKit Review Bot]

Comment on attachment 170070 [details]
Patch

Clearing flags on attachment: 170070

Committed r132237: <http://trac.webkit.org/changeset/132237>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.