Bug 165515

Summary: [Cocoa] NSAttributedString representation of text copied from -webkit-nbsp-mode:space element contains non-breaking space characters, but shouldn’t
Product: WebKit Reporter: mitz
Component: HTML EditingAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: ap, darin
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Possible fix (no tests and no change log)
none
Replace non-breaking spaces with spaces as needed
none
Replace non-breaking spaces with spaces as needed darin: review+

mitz
Reported 2016-12-06 21:55:12 PST
<rdar://problem/4108460> In content-editable elements, we use non-breaking space characters to prevent runs of spaces from collapsing. But when the text is copied, we should turn those back into spaces in the NSAttributedString pasteboard representation (like we already do in the plain-text representation).
Attachments
Possible fix (no tests and no change log) (1.65 KB, patch)
2016-12-06 21:56 PST, mitz
no flags
Replace non-breaking spaces with spaces as needed (10.17 KB, patch)
2016-12-10 18:20 PST, mitz
no flags
Replace non-breaking spaces with spaces as needed (4.56 KB, patch)
2016-12-10 19:07 PST, mitz
darin: review+
mitz
Comment 1 2016-12-06 21:56:39 PST
Created attachment 296374 [details] Possible fix (no tests and no change log)
mitz
Comment 2 2016-12-06 21:59:08 PST
Comment on attachment 296374 [details] Possible fix (no tests and no change log) View in context: https://bugs.webkit.org/attachment.cgi?id=296374&action=review > Source/WebCore/editing/cocoa/HTMLConverter.mm:2252 > + nbspMode = characterData.parentElement() ? static_cast<ENBSPMode>(downcast<CSSPrimitiveValue>(*_caches->computedStylePropertyForElement(*characterData.parentElement(), CSSPropertyWebkitNbspMode).get())) : NBNORMAL; Perhaps the chance of encountering an non-breaking space is low enough that we can use afford to use propertyValueForNode() and do a string compare instead of using computedStylePropertyForElement().
Darin Adler
Comment 3 2016-12-07 09:01:19 PST
Comment on attachment 296374 [details] Possible fix (no tests and no change log) View in context: https://bugs.webkit.org/attachment.cgi?id=296374&action=review >> Source/WebCore/editing/cocoa/HTMLConverter.mm:2252 >> + nbspMode = characterData.parentElement() ? static_cast<ENBSPMode>(downcast<CSSPrimitiveValue>(*_caches->computedStylePropertyForElement(*characterData.parentElement(), CSSPropertyWebkitNbspMode).get())) : NBNORMAL; > > Perhaps the chance of encountering an non-breaking space is low enough that we can use afford to use propertyValueForNode() and do a string compare instead of using computedStylePropertyForElement(). Seems clear we *could* do that even if non-breaking spaces are relatively common, because the textTransform logic below calls propertyValueForNode once per string already. So calling it a second time couldn’t make things more than twice as slow. I have to admit, I am not really an expert on the philosophy behind how the HTMLConverter works, but I see no harm in making this change.
Darin Adler
Comment 4 2016-12-08 09:07:58 PST
Change looks fine to me.
mitz
Comment 5 2016-12-10 18:20:58 PST
Created attachment 296837 [details] Replace non-breaking spaces with spaces as needed
mitz
Comment 6 2016-12-10 18:59:13 PST
Comment on attachment 296837 [details] Replace non-breaking spaces with spaces as needed Pleased to see that there’s a layout test that can tell the difference!
mitz
Comment 7 2016-12-10 19:07:33 PST
Created attachment 296839 [details] Replace non-breaking spaces with spaces as needed
mitz
Comment 8 2016-12-11 10:10:50 PST
Note You need to log in before you can comment on or make changes to this bug.