[Cocoa] NSAttributedString representation of text copied from -webkit-nbsp-mode:space element contains non-breaking space characters, but shouldn’t

• Component: HTML Editing
• Status: RESOLVED FIXED

<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).

Created attachment 296374 [details]
Possible fix (no tests and no change log)

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 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]

Change looks fine to me.

Created attachment 296837 [details]
Replace non-breaking spaces with spaces as needed

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!

Created attachment 296839 [details]
Replace non-breaking spaces with spaces as needed

Fixed in <https://trac.webkit.org/r209682>.