[webkit-reviews] review granted: [Bug 85864] [Shadow] TITLE elements in Shadow DOM should not affect document.title attribute : [Attachment 144267] Patch

Thu May 31 23:50:56 PDT 2012

Ryosuke Niwa <rniwa at webkit.org> has granted Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 85864: [Shadow] TITLE elements in Shadow DOM should not affect
document.title attribute
https://bugs.webkit.org/show_bug.cgi?id=85864

Attachment 144267: Patch
https://bugs.webkit.org/attachment.cgi?id=144267&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=144267&action=review

Please fix the test before you land & address morrita's comment.

> LayoutTests/fast/dom/shadow/title-element-in-shadow-expected.txt:4
> +PASS docForTest.title is ""
> +PASS docForTest.title is ""
> +PASS docForTest.title is ""
> +PASS docForTest.title is ""

This output is not helpful.

> LayoutTests/fast/dom/shadow/title-element-in-shadow.html:13
> +<!-- add newly created title element to the shadow root. -->

Could you at least put these descriptions in debug?

> LayoutTests/fast/dom/shadow/title-element-in-shadow.html:18
> +var shadowRoot = new WebKitShadowRoot(docForTest.documentElement);
> +var title = document.createElement('title');
> +title.innerHTML = 'shadow title';
> +shadowRoot.appendChild(title);
> +shouldBe('docForTest.title', '""');

Or more preferably, add some helper functions so that you can do something
along the line of:
shouldBe("shadowRoot.appendChild(createTitleElement('shadow title');
shadowRoot.document.title", '""');

> LayoutTests/fast/dom/shadow/title-element-in-shadow.html:23
> +<!-- add subtree which contains title element to the shadow root. -->
> +shadowRoot.innerHTML = '<HEAD><TITLE>shadow title</TITLE><HEAD>';
> +shouldBe('docForTest.title', '""');
> +shadowRoot.innerHTML = '';

Ditto.

> LayoutTests/fast/dom/shadow/title-element-in-shadow.html:29
> +<!-- add a subtree which has a shadow root, which has a title element as a
child. -->
> +shadowRoot.innerHTML = '<BODY><DIV id="div1"></DIV></BODY>';
> +var shadowRootForDiv = new
WebKitShadowRoot(shadowRoot.getElementById('div1'));
> +shadowRootForDiv.innerHTML = '<TITLE>shadow title</TITLE>';
> +shouldBe('docForTest.title', '""');

Ditto.