Bug 14062 – REGRESSION (r21212): "Remove Format" transforms spaces to U+00A0

• Component: HTML Editing
• Status: RESOLVED FIXED

[Alfonso Martínez de Lizarrondo]

Calling RemoveFormat on Safari transforms the spaces (U+0020) to NON-BREAK SPACE (U+00A0).

it's a regression in r21214. r21200 works fine

[Alfonso Martínez de Lizarrondo]

Created attachment 14925 [details]
Simplified testcase

This testcase shows the escaped contents of the document before and after calling RemoveFormat.

This one doesn't block FCKeditor anymore, as we have implemented a custom
formatting system. Please remove the block to 9915, as it is not anymore valid (I'm not able to do it).

[Alexey Proskuryakov]

<http://trac.webkit.org/projects/webkit/changeset/21212> is the only suspicious revision in this timeframe.

The attached test case also shows that innerHTML now ends with two %0A's, while there aren't any in the source.

[David Kilzer (:ddkilzer)]

<rdar://problem/5653315>

[Ryosuke Niwa]

Created attachment 31584 [details]
Undo the changeset 21212 since rebalanceWhitespaceAt has been fixed, the workaround introduced in 21212 is no longer needed

This patch changes the behavior of WebCore::InsertTextCommand::input.  It used to replace all spaces by   but after this patch is applied, WebCore::InsertTextCommand::input no longer replaces the spaces.  Since this would affect the way inserted text wraps, it may affect other third party applications or Web applications that assumes the existence of the bug.

This patch also replaces the several expected results.  This is due to the fact that those expected results had "unexpected" non-breaking spaces.  Since this patch fixes the problem, we must also fix those expected results.

[Ryosuke Niwa]

Created attachment 31587 [details]
Undo the changeset 21212

The previous patch contained an irrelevant change.

[Eric Seidel (no email)]

Comment on attachment 31587 [details]
Undo the changeset 21212

PrettyPatch seems to fail with this attachment.  I filed bug 26598.

[Eric Seidel (no email)]

This is really something for Justin to review.

[Ryosuke Niwa]

I forgot to mention that this patch lacks the fix for qt platform expected results.  I appreciate if someone could supply that later.

[Justin Garcia]

Comment on attachment 31587 [details]
Undo the changeset 21212

Nice work.  I like how your test case will run in FF too.  r=me

It looks like r21212 was a super aggressive workaround for https://bugs.webkit.org/show_bug.cgi?id=9441 which now appears to be fixed.  I'll close it out.

[Julie Parent]

I just went to commit this change, but I think something is wrong with the patch.  Specifically, it lists a file in the top level ChangeLog that is also in the WebCore/Changelog.  The entry in the top level ChangeLog shouldn't be there right?

[Ryosuke Niwa]

Created attachment 31749 [details]
fixes the bug, removed the ChangeLog change in the global directory

[Justin Garcia]

Comment on attachment 31749 [details]
fixes the bug, removed the ChangeLog change in the global directory

r=me

[Justin Garcia]

Comment on attachment 31587 [details]
Undo the changeset 21212

Ryosuke has a new patch.

[Julie Parent]

Commited as http://trac.webkit.org/changeset/45016

[Ryosuke Niwa]

Created attachment 31763 [details]
fix the expected result for  editing/execCommand/5142012-3.html

[Ryosuke Niwa]

(In reply to comment #16)
> Created an attachment (id=31763) [review]
> fix the expected result for  editing/execCommand/5142012-3.html
> 

Just note that since all my checkouts are broken, this patch hasn't been tested yet.

[Julie Parent]

Comment on attachment 31763 [details]
fix the expected result for  editing/execCommand/5142012-3.html

Marking this patch as obsolete - it was hand generated and not complete.  Ojan checked in the proper fix.

[Ojan Vafai]

Followup commit for layout test result: http://trac.webkit.org/changeset/45027