Bug 59305 - layoutTestController.dumpAsText should include text in shadow DOM
Summary: layoutTestController.dumpAsText should include text in shadow DOM
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dominic Cooney
URL:
Keywords:
Depends on:
Blocks: 72352 63602
  Show dependency treegraph
 
Reported: 2011-04-24 13:48 PDT by Dominic Cooney
Modified: 2012-07-19 01:56 PDT (History)
7 users (show)

See Also:


Attachments
Simple layout test for this (1.65 KB, patch)
2011-12-07 23:20 PST, Dominic Cooney
no flags Details | Formatted Diff | Diff
Test Patch (10.52 KB, patch)
2011-12-20 21:29 PST, Shinya Kawanaka
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 2011-04-24 13:48:56 PDT
The text output of layout tests does not include text in shadow DOM, but it should, for more convenient testing of shadow DOM.
Comment 1 Dimitri Glazkov (Google) 2011-04-24 13:57:39 PDT
weee!
Comment 2 Dimitri Glazkov (Google) 2011-05-27 14:36:57 PDT
This should be smart to ignore text inputs and other elements whose text dump didn't include the shadow DOM before.
Comment 3 Dominic Cooney 2011-12-07 23:20:50 PST
Created attachment 118336 [details]
Simple layout test for this
Comment 4 Dominic Cooney 2011-12-07 23:45:44 PST
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.?
Comment 5 Shinya Kawanaka 2011-12-19 22:31:10 PST
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?
Comment 6 Dominic Cooney 2011-12-20 14:50:35 PST
(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.
Comment 7 Shinya Kawanaka 2011-12-20 21:29:39 PST
Created attachment 120137 [details]
Test Patch
Comment 8 Shinya Kawanaka 2011-12-20 21:31:21 PST
(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.
Comment 9 WebKit Review Bot 2011-12-20 21:32:59 PST
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.
Comment 10 WebKit Review Bot 2011-12-20 21:33:24 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 11 WebKit Review Bot 2011-12-21 00:10:25 PST
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
Comment 12 Kent Tamura 2011-12-21 21:00:32 PST
(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.
Comment 13 Darin Fisher (:fishd, Google) 2011-12-22 09:13:05 PST
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.
Comment 14 Hajime Morrita 2012-07-19 01:56:08 PDT
Closing as WONTFIx per comment #12
We are writing shadow related tests even without this.