Bug 59305 – layoutTestController.dumpAsText should include text in shadow DOM

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Dominic Cooney]

The text output of layout tests does not include text in shadow DOM, but it should, for more convenient testing of shadow DOM.

[Dimitri Glazkov (Google)]

weee!

[Dimitri Glazkov (Google)]

This should be smart to ignore text inputs and other elements whose text dump didn't include the shadow DOM before.

[Dominic Cooney]

Created attachment 118336 [details]
Simple layout test for this

[Dominic Cooney]

It looks like dumpAsText iterates over frames and prints the innerText. Since we don’t want to change innerText, I guess we need something that uses TextIterator etc.?

[Shinya Kawanaka]

I was extending TextIterator to handle this, but I found that a shadow content element does not have a renderer. So I cannot emit 'Light', though I can emit 'Shadow A' and 'Shadow B'.

How do I add a renderer to shadow content element?

[Dominic Cooney]

(In reply to comment #5)
> How do I add a renderer to shadow content element?

Shadow content element should not have a renderer because it is not rendered. Content element has a list of inclusions which you could walk to get the elements output at that point.

You should be careful not to change the behavior of eg textContent when implementing this in DRT.

hayato is working on visual tree iteration including shadow/content; you should be able to use the API he is designing.

[Shinya Kawanaka]

Created attachment 120137 [details]
Test Patch

[Shinya Kawanaka]

(In reply to comment #7)
> Created an attachment (id=120137) [details]
> Patch

I've uploaded a patch which does not consider ShadowContentElement, and only works on chromium. I think we can use it for discussing the problem.

[WebKit Review Bot]

Attachment 120137 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebCore/editing/TextIterator.cpp:400:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[WebKit Review Bot]

Comment on attachment 120137 [details]
Test Patch

Attachment 120137 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10993515

New failing tests:
http/tests/inspector-enabled/dedicated-workers-list.html
http/tests/media/reload-after-dialog.html
fast/dom/shadow/activeelement-should-be-shadowhost.html
http/tests/inspector/resource-tree/resource-tree-document-url.html
http/tests/security/local-video-poster-from-remote.html
http/tests/security/local-video-source-from-remote.html
fast/dom/HTMLProgressElement/progress-element-form.html
compositing/geometry/clipped-video-controller.html
http/tests/media/text-served-as-text.html
http/tests/media/video-served-as-text.html
http/tests/media/pdf-served-as-pdf.html
fast/css/first-letter-block-form-controls-crash.html
fast/dom/shadow/layout-tests-can-access-shadow.html
fast/dom/HTMLMeterElement/meter-percent-size.html
fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
accessibility/anchor-linked-anonymous-block-crash.html
http/tests/security/local-video-src-from-remote.html
fast/dom/HTMLProgressElement/indeterminate-progress-002.html
http/tests/media/video-error-abort.html

[Kent Tamura]

(In reply to comment #11)

As you see in comment #11, the patch will break many existing tests.
Adding Chromium-specific test results for these tests is not acceptable.  We can't change the default behavior until all of platforms implement it.

[Darin Fisher (:fishd, Google)]

Comment on attachment 120137 [details]
Test Patch

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

> Source/WebKit/chromium/public/WebElement.h:62
> +        WEBKIT_EXPORT WebString innerTextWithShadow();

LGTM for WebKit API change.

[Hajime Morrita]

Closing as WONTFIx per comment #12
We are writing shadow related tests even without this.