⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 247720 in webkit


Ignore:
Timestamp:
Jul 23, 2019, 12:21:29 AM (7 years ago)
Author:
rniwa@webkit.org
Message:

WebKit should strip away system font names from the pasted content
​https://bugs.webkit.org/show_bug.cgi?id=199975
<rdar://problem/53336353>

Reviewed by Darin Adler.

Source/WebCore:

Cocoa HTML Writer sometimes generate system font names such as ".AppleSystemUIFont", ".SFUI-Regular", and ".SF UI Mono".
We need to strip away these font names upon paste to avoid these font names falling back to Times New Roman.

Added the code to strip these font names away in EditingStyle::mergeStyleFromRulesForSerialization, which is used by
StylizedMarkupAccumulator to generate HTML during copy. This works because WebContentReader::readWebArchive invokes
sanitizeMarkupWithArchive which inserts the pasteboard content into a temporary document then re-serializes back to HTML
using StylizedMarkupAccumulator before the actual pasting happens.

This approach has a few benefits over stripping away these font names in ReplaceSelectionCommand:

  1. It would only affect clients that opts-in to copy & paste sanitization. e.g. it won't affect legacy WebKit clients

and those that opt out of pasteboard content sanitization.

  1. It preserves font names such as ".SF Blah" that a website may insert as some kind of house keeping purposes if ever.

While we don't have any evidence that there is any such a website but it's a real risk nonetheless. The copy side fix would
only affect cross-site and cross-app pasting, which is rare and less likely to affect real user scenarios.

  1. It avoids exposing bogus .Apple* or .SF* font names to websites that directly use event.clipboardData.getData.

Indeed stripping away bogus markup like this is one of the key features / benefit of using copy & paste sanitization.

Test: editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts.html

  • editing/EditingStyle.cpp:

(WebCore::usesForbiddenSystemFontAsOnlyFontFamilyName): Added.
(WebCore::EditingStyle::mergeStyleFromRulesForSerialization): Added the code to remove font-family property when needed.

  • platform/graphics/FontCache.h:
  • platform/graphics/cocoa/FontCacheCoreText.cpp:

(WebCore::isSystemFont): Moved.
(WebCore::FontCache::isSystemFontForbiddenForEditing): Added.

  • platform/graphics/freetype/FontCacheFreeType.cpp:

(WebCore::FontCache::isSystemFontForbiddenForEditing): Added. Always returns false.

  • platform/graphics/win/FontCacheWin.cpp:

(WebCore::FontCache::isSystemFontForbiddenForEditing): Ditto.

Tools:

Added a test to strip away system font names such as ".AppleSystemUIFont", ".SFUI-Regular", and ".SF UI Mono".

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:
  • TestWebKitAPI/Tests/WebKitCocoa/cocoa-writer-markup-with-system-fonts.html: Added.

LayoutTests:

Added a test for ClipboardData.getData returning the original markup and execCommand('insertHTML', ~)
not stripping away system font names.

  • editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts-expected.txt: Added.
  • editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts.html: Added.
Location:
trunk
Files:
3 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247712 r247720  
     12019-07-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        WebKit should strip away system font names from the pasted content
     4        https://bugs.webkit.org/show_bug.cgi?id=199975
     5        <rdar://problem/53336353>
     6
     7        Reviewed by Darin Adler.
     8
     9        Added a test for ClipboardData.getData returning the original markup and execCommand('insertHTML', ~)
     10        not stripping away system font names.
     11
     12        * editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts-expected.txt: Added.
     13        * editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts.html: Added.
     14
    1152019-07-22  Simon Fraser  <simon.fraser@apple.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r247714 r247720  
     12019-07-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        WebKit should strip away system font names from the pasted content
     4        https://bugs.webkit.org/show_bug.cgi?id=199975
     5        <rdar://problem/53336353>
     6
     7        Reviewed by Darin Adler.
     8
     9        Cocoa HTML Writer sometimes generate system font names such as ".AppleSystemUIFont", ".SFUI-Regular", and ".SF UI Mono".
     10        We need to strip away these font names upon paste to avoid these font names falling back to Times New Roman.
     11
     12        Added the code to strip these font names away in EditingStyle::mergeStyleFromRulesForSerialization, which is used by
     13        StylizedMarkupAccumulator to generate HTML during copy. This works because WebContentReader::readWebArchive invokes
     14        sanitizeMarkupWithArchive which inserts the pasteboard content into a temporary document then re-serializes back to HTML
     15        using StylizedMarkupAccumulator before the actual pasting happens.
     16
     17        This approach has a few benefits over stripping away these font names in ReplaceSelectionCommand:
     18
     19        1. It would only affect clients that opts-in to copy & paste sanitization. e.g. it won't affect legacy WebKit clients
     20        and those that opt out of pasteboard content sanitization.
     21
     22        2. It preserves font names such as ".SF Blah" that a website may insert as some kind of house keeping purposes if ever.
     23        While we don't have any evidence that there is any such a website but it's a real risk nonetheless. The copy side fix would
     24        only affect cross-site and cross-app pasting, which is rare and less likely to affect real user scenarios.
     25
     26        3. It avoids exposing bogus .Apple* or .SF* font names to websites that directly use event.clipboardData.getData.
     27        Indeed stripping away bogus markup like this is one of the key features / benefit of using copy & paste sanitization.
     28
     29        Test: editing/pasteboard/paste-cocoa-writer-markup-with-system-fonts.html
     30
     31        * editing/EditingStyle.cpp:
     32        (WebCore::usesForbiddenSystemFontAsOnlyFontFamilyName): Added.
     33        (WebCore::EditingStyle::mergeStyleFromRulesForSerialization): Added the code to remove font-family property when needed.
     34        * platform/graphics/FontCache.h:
     35        * platform/graphics/cocoa/FontCacheCoreText.cpp:
     36        (WebCore::isSystemFont): Moved.
     37        (WebCore::FontCache::isSystemFontForbiddenForEditing): Added.
     38        * platform/graphics/freetype/FontCacheFreeType.cpp:
     39        (WebCore::FontCache::isSystemFontForbiddenForEditing): Added. Always returns false.
     40        * platform/graphics/win/FontCacheWin.cpp:
     41        (WebCore::FontCache::isSystemFontForbiddenForEditing): Ditto.
     42
    1432019-07-22  Yusuke Suzuki  <ysuzuki@apple.com>
    244

  • trunk/Source/WebCore/editing/EditingStyle.cpp

    r246490 r247720  
    3030#include "ApplyStyleCommand.h"
    3131#include "CSSComputedStyleDeclaration.h"
     32#include "CSSFontFamily.h"
    3233#include "CSSFontStyleValue.h"
    3334#include "CSSParser.h"
    … …  
    3839#include "Editing.h"
    3940#include "Editor.h"
     41#include "FontCache.h"
     42#include "FontCascade.h"
    4043#include "Frame.h"
    4144#include "HTMLFontElement.h"
    … …  
    12801283}
    12811284
     1285static bool usesForbiddenSystemFontAsOnlyFontFamilyName(CSSValue& value)
     1286{
     1287    if (!is<CSSValueList>(value) || downcast<CSSValueList>(value).length() != 1)
     1288        return false;
     1289
     1290    auto& item = *downcast<CSSValueList>(value).item(0);
     1291    if (!is<CSSPrimitiveValue>(item))
     1292        return false;
     1293
     1294    auto& primitiveValue = downcast<CSSPrimitiveValue>(item);
     1295    if (!primitiveValue.isFontFamily())
     1296        return false;
     1297    return FontCache::isSystemFontForbiddenForEditing(primitiveValue.fontFamily().familyName);
     1298}
     1299
    12821300void EditingStyle::mergeStyleFromRulesForSerialization(StyledElement& element)
    12831301{
    … …  
    12901308    ComputedStyleExtractor computedStyle(&element);
    12911309
     1310    bool shouldRemoveFontFamily = false;
    12921311    {
    12931312        unsigned propertyCount = m_mutableStyle->propertyCount();
    12941313        for (unsigned i = 0; i < propertyCount; ++i) {
    12951314            StyleProperties::PropertyReference property = m_mutableStyle->propertyAt(i);
    1296             CSSValue* value = property.value();
    1297             if (!is<CSSPrimitiveValue>(*value))
     1315            CSSValue& value = *property.value();
     1316            if (property.id() == CSSPropertyFontFamily && usesForbiddenSystemFontAsOnlyFontFamilyName(value)) {
     1317                shouldRemoveFontFamily = true;
    12981318                continue;
    1299             if (downcast<CSSPrimitiveValue>(*value).isPercentage()) {
     1319            }
     1320            if (!is<CSSPrimitiveValue>(value))
     1321                continue;
     1322            if (downcast<CSSPrimitiveValue>(value).isPercentage()) {
    13001323                if (auto computedPropertyValue = computedStyle.propertyValue(property.id()))
    13011324                    fromComputedStyle->addParsedProperty(CSSProperty(property.id(), WTFMove(computedPropertyValue)));
    13021325            }
    13031326        }
     1327    }
     1328    if (shouldRemoveFontFamily) {
     1329        m_mutableStyle->removeProperty(CSSPropertyFontFamily);
     1330        fromComputedStyle->removeProperty(CSSPropertyFontFamily);
    13041331    }
    13051332    m_mutableStyle->mergeAndOverrideOnConflict(fromComputedStyle.get());

  • trunk/Source/WebCore/platform/graphics/FontCache.h

    r247498 r247720  
    198198    void platformInit();
    199199
     200    static bool isSystemFontForbiddenForEditing(const String&);
     201
    200202#if PLATFORM(COCOA)
    201203    WEBCORE_EXPORT static void setFontWhitelist(const Vector<String>&);

  • trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp

    r247699 r247720  
    786786}
    787787
     788static inline bool isSystemFont(const String& family)
     789{
     790    // AtomString's operator[] handles out-of-bounds by returning 0.
     791    return family[0] == '.';
     792}
     793
     794bool FontCache::isSystemFontForbiddenForEditing(const String& fontFamily)
     795{
     796    return isSystemFont(fontFamily);
     797}
     798
    788799static CTFontSymbolicTraits computeTraits(const FontDescription& fontDescription)
    789800{
    … …  
    831842    for (auto& item : inputWhitelist)
    832843        whitelist.add(item);
    833 }
    834 
    835 static inline bool isSystemFont(const AtomString& family)
    836 {
    837     // AtomString's operator[] handles out-of-bounds by returning 0.
    838     return family[0] == '.';
    839844}
    840845
    … …  
    11861191{
    11871192    const auto& whitelist = fontWhitelist();
    1188     if (!isSystemFont(family) && whitelist.size() && !whitelist.contains(family))
     1193    if (!isSystemFont(family.string()) && whitelist.size() && !whitelist.contains(family))
    11891194        return { nullptr };
    11901195

  • trunk/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp

    r246490 r247720  
    187187}
    188188
     189bool FontCache::isSystemFontForbiddenForEditing(const String&)
     190{
     191    return false;
     192}
     193
    189194Ref<Font> FontCache::lastResortFallbackFont(const FontDescription& fontDescription)
    190195{

  • trunk/Source/WebCore/platform/graphics/win/FontCacheWin.cpp

    r246490 r247720  
    329329}
    330330
     331bool FontCache::isSystemFontForbiddenForEditing(const String&)
     332{
     333    return false;
     334}
     335
    331336RefPtr<Font> FontCache::fontFromDescriptionAndLogFont(const FontDescription& fontDescription, const LOGFONT& font, AtomString& outFontFamilyName)
    332337{

  • trunk/Tools/ChangeLog

    r247710 r247720  
     12019-07-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        WebKit should strip away system font names from the pasted content
     4        https://bugs.webkit.org/show_bug.cgi?id=199975
     5        <rdar://problem/53336353>
     6
     7        Reviewed by Darin Adler.
     8
     9        Added a test to strip away system font names such as ".AppleSystemUIFont", ".SFUI-Regular", and ".SF UI Mono".
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:
     13        * TestWebKitAPI/Tests/WebKitCocoa/cocoa-writer-markup-with-system-fonts.html: Added.
     14
    1152019-07-22  Aakash Jain  <aakash_jain@apple.com>
    216

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r247702 r247720  
    707707                9B7D740F1F8378770006C432 /* paste-rtfd.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B7D740E1F8377E60006C432 /* paste-rtfd.html */; };
    708708                9BAD7F3E22690F2000F8DA66 /* DeallocWebViewInEventListener.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9BAD7F3D22690F1400F8DA66 /* DeallocWebViewInEventListener.mm */; };
     709                9BAE177B22E2BBFB00DF3098 /* cocoa-writer-markup-with-system-fonts.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9BAE177A22E2BB6B00DF3098 /* cocoa-writer-markup-with-system-fonts.html */; };
    709710                9BCB7C2820130600003E7C0C /* PasteHTML.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9BCB7C2620130600003E7C0C /* PasteHTML.mm */; };
    710711                9BCD411A206DBCA3001D71BE /* mso-list-on-h4.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9BCD4119206D5ED7001D71BE /* mso-list-on-h4.html */; };
    … …  
    10761077                        dstSubfolderSpec = 7;
    10771078                        files = (
     1079                                9BAE177B22E2BBFB00DF3098 /* cocoa-writer-markup-with-system-fonts.html in Copy Resources */,
    10781080                                55A817FF2181021A0004A39A /* 100x100-red.tga in Copy Resources */,
    10791081                                1A9E52C913E65EF4006917F5 /* 18-characters.html in Copy Resources */,
    … …  
    20352037                9B7D740E1F8377E60006C432 /* paste-rtfd.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "paste-rtfd.html"; sourceTree = "<group>"; };
    20362038                9BAD7F3D22690F1400F8DA66 /* DeallocWebViewInEventListener.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeallocWebViewInEventListener.mm; sourceTree = "<group>"; };
     2039                9BAE177A22E2BB6B00DF3098 /* cocoa-writer-markup-with-system-fonts.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "cocoa-writer-markup-with-system-fonts.html"; sourceTree = "<group>"; };
    20372040                9BCB7C2620130600003E7C0C /* PasteHTML.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = PasteHTML.mm; sourceTree = "<group>"; };
    20382041                9BCD4119206D5ED7001D71BE /* mso-list-on-h4.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "mso-list-on-h4.html"; sourceTree = "<group>"; };
    … …  
    32603263                                5120C83B1E674E350025B250 /* WebsiteDataStoreCustomPaths.html */,
    32613264                                2E131C171D83A97E001BA36C /* wide-autoplaying-video-with-audio.html */,
     3265                                9BAE177A22E2BB6B00DF3098 /* cocoa-writer-markup-with-system-fonts.html */,
    32623266                        );
    32633267                        name = Resources;

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm

    r242339 r247720  
    328328}
    329329
     330TEST(PasteHTML, StripsSystemFontNames)
     331{
     332    writeHTMLToPasteboard([NSString stringWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"cocoa-writer-markup-with-system-fonts" ofType:@"html" inDirectory:@"TestWebKitAPI.resources"] encoding:NSUTF8StringEncoding error:NULL]);
     333
     334    auto webView = createWebViewWithCustomPasteboardDataSetting(true);
     335    [webView synchronouslyLoadTestPageNamed:@"paste-rtfd"];
     336    [webView paste:nil];
     337
     338    EXPECT_WK_STREQ("[\"text/html\"]", [webView stringByEvaluatingJavaScript:@"JSON.stringify(clipboardData.types)"]);
     339    [webView stringByEvaluatingJavaScript:@"window.htmlInDataTransfer = clipboardData.values[0]"];
     340    [webView stringByEvaluatingJavaScript:@"window.pastedHTML = editor.innerHTML"];
     341
     342    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"pastedHTML.includes('Hello Cocoa')"].boolValue);
     343    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"pastedHTML.includes('font-weight: bold')"].boolValue);
     344    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!pastedHTML.includes('.AppleSystemUIFont')"].boolValue);
     345    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!pastedHTML.includes('.SFUI')"].boolValue);
     346    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!pastedHTML.includes('.SF')"].boolValue);
     347
     348    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"htmlInDataTransfer.includes('Hello Cocoa')"].boolValue);
     349    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"htmlInDataTransfer.includes('font-weight: bold')"].boolValue);
     350    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!htmlInDataTransfer.includes('.AppleSystemUIFont')"].boolValue);
     351    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!htmlInDataTransfer.includes('.SFUI')"].boolValue);
     352    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!htmlInDataTransfer.includes('.SF')"].boolValue);
     353
     354    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"getComputedStyle(document.querySelector('.s2')).fontFamily"],
     355        [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.body).fontFamily"]);
     356    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"getComputedStyle(document.querySelector('.s4')).fontFamily"],
     357        [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.body).fontFamily"]);
     358}
    330359
    331360#endif // PLATFORM(COCOA)
Note: See TracChangeset for help on using the changeset viewer.