Bug 164578

Summary:

Copied text would contain text inside CDATA sections and comments

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, darin, enrica, rniwa, sam, zcorpan

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

164501

Bug Blocks:

Attachments:

Description	Flags
Fixes the bug	none
Archive of layout-test-results from ews101 for mac-yosemite	none
Patch for landing	none

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	Details Formatted Diff Diff
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	Details Formatted Diff Diff
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.