RESOLVED FIXED164578
Copied text would contain text inside CDATA sections and comments 
https://bugs.webkit.org/show_bug.cgi?id=164578
Summary Copied text would contain text inside CDATA sections and comments
Ryosuke Niwa
Reported 2016-11-09 20:02:51 PST
WebKit currently serializes text inside comments and CDATA sections but we shouldn't. <rdar://problem/19834542>
Attachments
Fixes the bug (4.21 KB, patch)
2016-11-10 01:04 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews101 for mac-yosemite (941.97 KB, application/zip)
2016-11-10 02:03 PST, Build Bot 		no flags
Details
Patch for landing (5.39 KB, patch)
2016-11-10 13:02 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2016-11-10 01:04:18 PST
Created attachment 294343 [details] Fixes the bug
Build Bot
Comment 2 2016-11-10 02:02:57 PST
Comment on attachment 294343 [details] Fixes the bug Attachment 294343 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2488160 New failing tests: editing/mac/attributed-string/comment-cdata-section.html
Build Bot
Comment 3 2016-11-10 02:03:00 PST
Created attachment 294350 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Darin Adler
Comment 4 2016-11-10 09:03:13 PST
Won’t review yet: The new test is failing on the bot. It seems peculiar to always skip CDATA; can’t those be rendered as text?
Darin Adler
Comment 5 2016-11-10 09:08:44 PST
Comment on attachment 294343 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=294343&action=review Not sure why the test is failing. > Source/WebCore/ChangeLog:12 > + In the long term, we should check the visibility of the text as done in MarkupAccumulator. Any chance that long term we could TextIterator to convert to attributed text, instead of a separate algorithm that tries to accomplish the same thing?
Ryosuke Niwa
Comment 6 2016-11-10 12:36:44 PST
(In reply to comment #5) > Comment on attachment 294343 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294343&action=review > > Not sure why the test is failing. > > > Source/WebCore/ChangeLog:12 > > + In the long term, we should check the visibility of the text as done in MarkupAccumulator. > > Any chance that long term we could TextIterator to convert to attributed > text, instead of a separate algorithm that tries to accomplish the same > thing? I think in order to do that, TextIterator needs to support emitting table & list because attributed string today supports those two features.
Ryosuke Niwa
Comment 7 2016-11-10 12:59:58 PST
(In reply to comment #4) > Won’t review yet: The new test is failing on the bot. It seems peculiar to > always skip CDATA; can’t those be rendered as text? No, CDATASections are like comments. They're never rendered.
Ryosuke Niwa
Comment 8 2016-11-10 13:01:01 PST
(In reply to comment #5) > Comment on attachment 294343 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294343&action=review > > Not sure why the test is failing. I think Yosemite needs its own result due to formatting difference: @@ -15,13 +15,11 @@ LineBreakMode: 0 Tabs: () DefaultTabInterval: 36 - Blocks: ( -) - Lists: ( -) + Blocks: (null) + Lists: (null) BaseWritingDirection: 0 HyphenationFactor: 0 - TighteningForTruncation: YES + TighteningFactor: 0.05 HeaderLevel: 0 [hello world. ] NSFont: Times-Roman 16.00 pt.
Ryosuke Niwa
Comment 9 2016-11-10 13:02:45 PST
Created attachment 294398 [details] Patch for landing
Ryosuke Niwa
Comment 10 2016-11-10 13:07:19 PST
Comment on attachment 294398 [details] Patch for landing Wait for EWS
WebKit Commit Bot
Comment 11 2016-11-10 14:24:40 PST
Comment on attachment 294398 [details] Patch for landing Clearing flags on attachment: 294398 Committed r208565: <http://trac.webkit.org/changeset/208565>
WebKit Commit Bot
Comment 12 2016-11-10 14:24:44 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 13 2016-11-10 20:36:09 PST
(In reply to comment #6) > I think in order to do that, TextIterator needs to support emitting table & > list because attributed string today supports those two features. Sure, the code using TextIterator needs to emit table and list. I think that can be built based on the ability to inspect the element that the text is inside, which is part of TextIterator's interface.
Simon Pieters (:zcorpan)
Comment 14 2016-12-07 13:04:58 PST
> No, CDATASections are like comments. They're never rendered. No? They're like Text nodes. They're rendered. data:text/xml,<html%20xmlns="http://www.w3.org/1999/xhtml"><body>Hello%20<![CDATA[%20World%20]]></body></html>
Note You need to log in before you can comment on or make changes to this bug.