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

Changeset 249707 in webkit


Ignore:
Timestamp:
Sep 9, 2019, 10:30:56 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r249535. rdar://problem/55183140

Location:
branches/safari-608-branch
Files:
3 added
8 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608-branch/LayoutTests/ChangeLog

    r249698 r249707  
     12019-09-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (iOS 13): Bulleted list copied from Notes to Mail results in Times New Roman
     4        https://bugs.webkit.org/show_bug.cgi?id=201490
     5
     6        Reviewed by Daniel Bates.
     7
     8        Added a test to make sure -webkit-standard font family name isn't stripped away when sanitization is not in effect.
     9
     10        * editing/pasteboard/paste-cocoa-writer-markup-with-webkit-standard-font-family-expected.txt: Added.
     11        * editing/pasteboard/paste-cocoa-writer-markup-with-webkit-standard-font-family.html: Added.
     12
    1132019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    214

  • branches/safari-608-branch/Source/WebCore/ChangeLog

    r249698 r249707  
     12019-09-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (iOS 13): Bulleted list copied from Notes to Mail results in Times New Roman
     4        https://bugs.webkit.org/show_bug.cgi?id=201490
     5
     6        Reviewed by Daniel Bates.
     7
     8        The bug was caused by an element in the pasted content not having any explicit font name resolving
     9        to use the font-family value of `-webkit-standard`. When such an inline style is inserted into
     10        Mail's WKWebView which sets a different font family, ReplaceSelectionCommand would fail to strip away,
     11        making the pasted content using the default font family of Times New Roman.
     12
     13        Fixed the bug by stripping away font-family set to -webkit-standard in the sanitization document
     14        since that's indicative of the pasted content not having any font family being specified.
     15
     16        In the future, we should consider making regular copy (as opposed to the copy for sanitization)
     17        resolve generic font family names to concrete font names since different WKWebView might be using
     18        different concrete font names. Unfortuantely, such a change is quite involved and risky since various
     19        paste side code in EditingStyle that removes redundant inline styles (i.e. redundant `font-family`)
     20        need to be aware of this special font family resolution.
     21
     22        Tests: editing/pasteboard/paste-cocoa-writer-markup-with-webkit-standard-font-family.html
     23               PasteHTML.DoesNotAddStandardFontFamily
     24
     25        * editing/EditingStyle.cpp:
     26        (WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
     27        (WebCore::EditingStyle::wrappingStyleForSerialization):
     28        (WebCore::familyNameFromCSSPrimitiveValue): Added.
     29        (WebCore::loneFontFamilyName): Extracted from usesForbiddenSystemFontAsOnlyFontFamilyName. Fixed
     30        a bug that it was not handling the case when `font-family` property's value is a CSSPrimitiveValue
     31        instead of a CSSValueList.
     32        (WebCore::usesForbiddenSystemFontAsOnlyFontFamilyName): Deleted.
     33        (WebCore::EditingStyle::mergeStyleFromRulesForSerialization): Remove `font-family` property when
     34        StandardFontFamilySerializationMode::Strip is specified and its value is `-webkit-standard`.
     35        * editing/EditingStyle.h:
     36        * editing/markup.cpp:
     37        (WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator): Added
     38        StandardFontFamilySerializationMode as an argument.
     39        (WebCore::StyledMarkupAccumulator::appendStartTag):
     40        (WebCore::StyledMarkupAccumulator::serializeNodes):
     41        (WebCore::serializePreservingVisualAppearanceInternal): Ditto.
     42        (WebCore::serializePreservingVisualAppearance): Use StandardFontFamilySerializationMode::Keep
     43        to preserve the pre-existing behavior.
     44        (WebCore::sanitizedMarkupForFragmentInDocument): Use StandardFontFamilySerializationMode::Strip
     45        as this is the code used by sanitization code.
     46
    1472019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    248

  • branches/safari-608-branch/Source/WebCore/editing/EditingStyle.cpp

    r247966 r249707  
    11581158}
    11591159
    1160 void EditingStyle::mergeInlineAndImplicitStyleOfElement(StyledElement& element, CSSPropertyOverrideMode mode, PropertiesToInclude propertiesToInclude)
     1160void EditingStyle::mergeInlineAndImplicitStyleOfElement(StyledElement& element, CSSPropertyOverrideMode mode, PropertiesToInclude propertiesToInclude, StandardFontFamilySerializationMode standardFontFamilySerializationMode)
    11611161{
    11621162    auto styleFromRules = EditingStyle::create();
    1163     styleFromRules->mergeStyleFromRulesForSerialization(element);
     1163    styleFromRules->mergeStyleFromRulesForSerialization(element, standardFontFamilySerializationMode);
    11641164
    11651165    if (element.inlineStyle())
    … …  
    11821182}
    11831183
    1184 Ref<EditingStyle> EditingStyle::wrappingStyleForSerialization(Node& context, bool shouldAnnotate)
     1184Ref<EditingStyle> EditingStyle::wrappingStyleForSerialization(Node& context, bool shouldAnnotate, StandardFontFamilySerializationMode standardFontFamilySerializationMode)
    11851185{
    11861186    if (shouldAnnotate) {
    … …  
    12031203    for (Node* node = &context; node && !node->isDocumentNode(); node = node->parentNode()) {
    12041204        if (is<StyledElement>(*node) && !isMailBlockquote(node))
    1205             wrappingStyle->mergeInlineAndImplicitStyleOfElement(downcast<StyledElement>(*node), EditingStyle::DoNotOverrideValues, EditingStyle::EditingPropertiesInEffect);
     1205            wrappingStyle->mergeInlineAndImplicitStyleOfElement(downcast<StyledElement>(*node), DoNotOverrideValues, EditingPropertiesInEffect, standardFontFamilySerializationMode);
    12061206    }
    12071207
    … …  
    12831283}
    12841284
    1285 static bool usesForbiddenSystemFontAsOnlyFontFamilyName(CSSValue& value)
    1286 {
     1285static String familyNameFromCSSPrimitiveValue(const CSSPrimitiveValue& primitiveValue)
     1286{
     1287    if (!primitiveValue.isFontFamily())
     1288        return { };
     1289    return primitiveValue.fontFamily().familyName;
     1290}
     1291
     1292static String loneFontFamilyName(const CSSValue& value)
     1293{
     1294    if (is<CSSPrimitiveValue>(value))
     1295        return familyNameFromCSSPrimitiveValue(downcast<CSSPrimitiveValue>(value));
     1296
    12871297    if (!is<CSSValueList>(value) || downcast<CSSValueList>(value).length() != 1)
    1288         return false;
     1298        return { };
    12891299
    12901300    auto& item = *downcast<CSSValueList>(value).item(0);
    12911301    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 
    1300 void EditingStyle::mergeStyleFromRulesForSerialization(StyledElement& element)
     1302        return { };
     1303
     1304    return familyNameFromCSSPrimitiveValue(downcast<CSSPrimitiveValue>(item));
     1305}
     1306
     1307void EditingStyle::mergeStyleFromRulesForSerialization(StyledElement& element, StandardFontFamilySerializationMode standardFontFamilySerializationMode)
    13011308{
    13021309    mergeStyleFromRules(element);
    … …  
    13141321            StyleProperties::PropertyReference property = m_mutableStyle->propertyAt(i);
    13151322            CSSValue& value = *property.value();
    1316             if (property.id() == CSSPropertyFontFamily && usesForbiddenSystemFontAsOnlyFontFamilyName(value)) {
    1317                 shouldRemoveFontFamily = true;
     1323            if (property.id() == CSSPropertyFontFamily) {
     1324                auto familyName = loneFontFamilyName(value);
     1325                if (FontCache::isSystemFontForbiddenForEditing(familyName)
     1326                    || (standardFontFamilySerializationMode == StandardFontFamilySerializationMode::Strip && familyName == standardFamily))
     1327                    shouldRemoveFontFamily = true;
    13181328                continue;
    13191329            }

  • branches/safari-608-branch/Source/WebCore/editing/EditingStyle.h

    r242776 r249707  
    6161enum class TextDecorationChange { None, Add, Remove };
    6262
     63// FIXME: "Keep" should be "Resolve" instead and resolve all generic font family names.
     64enum class StandardFontFamilySerializationMode : uint8_t { Keep, Strip };
     65
    6366class EditingStyle : public RefCounted<EditingStyle> {
    6467public:
    … …  
    142145    enum CSSPropertyOverrideMode { OverrideValues, DoNotOverrideValues };
    143146    void mergeInlineStyleOfElement(StyledElement&, CSSPropertyOverrideMode, PropertiesToInclude = AllProperties);
    144     static Ref<EditingStyle> wrappingStyleForSerialization(Node& context, bool shouldAnnotate);
     147    static Ref<EditingStyle> wrappingStyleForSerialization(Node& context, bool shouldAnnotate, StandardFontFamilySerializationMode);
    145148    void mergeStyleFromRules(StyledElement&);
    146     void mergeStyleFromRulesForSerialization(StyledElement&);
     149    void mergeStyleFromRulesForSerialization(StyledElement&, StandardFontFamilySerializationMode);
    147150    void removeStyleFromRulesAndContext(StyledElement&, Node* context);
    148151    void removePropertiesInElementDefaultStyle(Element&);
    … …  
    182185    template<typename T> TriState triStateOfStyle(T& styleToCompare, ShouldIgnoreTextOnlyProperties) const;
    183186    bool conflictsWithInlineStyleOfElement(StyledElement&, RefPtr<MutableStyleProperties>* newInlineStyle, EditingStyle* extractedStyle) const;
    184     void mergeInlineAndImplicitStyleOfElement(StyledElement&, CSSPropertyOverrideMode, PropertiesToInclude);
     187    void mergeInlineAndImplicitStyleOfElement(StyledElement&, CSSPropertyOverrideMode, PropertiesToInclude, StandardFontFamilySerializationMode);
    185188    void mergeStyle(const StyleProperties*, CSSPropertyOverrideMode);
    186189

  • branches/safari-608-branch/Source/WebCore/editing/markup.cpp

    r247222 r249707  
    223223
    224224    StyledMarkupAccumulator(const Position& start, const Position& end, Vector<Node*>* nodes, ResolveURLs, SerializeComposedTree,
    225         AnnotateForInterchange, MSOListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized = nullptr);
     225        AnnotateForInterchange, StandardFontFamilySerializationMode, MSOListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized = nullptr);
    226226
    227227    Node* serializeNodes(const Position& start, const Position& end);
    … …  
    319319    bool m_useComposedTree;
    320320    bool m_needsPositionStyleConversion;
     321    StandardFontFamilySerializationMode m_standardFontFamilySerializationMode;
     322    bool m_shouldPreserveMSOList;
    321323    bool m_needRelativeStyleWrapper { false };
    322324    bool m_needClearingDiv { false };
    323     bool m_shouldPreserveMSOList;
    324325    bool m_inMSOList { false };
    325326};
    326327
    327328inline StyledMarkupAccumulator::StyledMarkupAccumulator(const Position& start, const Position& end, Vector<Node*>* nodes, ResolveURLs urlsToResolve, SerializeComposedTree serializeComposedTree,
    328     AnnotateForInterchange annotate, MSOListMode msoListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized)
     329    AnnotateForInterchange annotate, StandardFontFamilySerializationMode standardFontFamilySerializationMode, MSOListMode msoListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized)
    329330    : MarkupAccumulator(nodes, urlsToResolve)
    330331    , m_start(start)
    … …  
    334335    , m_useComposedTree(serializeComposedTree == SerializeComposedTree::Yes)
    335336    , m_needsPositionStyleConversion(needsPositionStyleConversion)
     337    , m_standardFontFamilySerializationMode(standardFontFamilySerializationMode)
    336338    , m_shouldPreserveMSOList(msoListMode == MSOListMode::Preserve)
    337339{
    … …  
    532534        if (shouldAnnotateOrForceInline) {
    533535            if (shouldAnnotate())
    534                 newInlineStyle->mergeStyleFromRulesForSerialization(downcast<HTMLElement>(*const_cast<Element*>(&element)));
     536                newInlineStyle->mergeStyleFromRulesForSerialization(downcast<HTMLElement>(*const_cast<Element*>(&element)), m_standardFontFamilySerializationMode);
    535537
    536538            if (addDisplayInline)
    … …  
    580582
    581583    if (m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode())
    582         m_wrappingStyle = EditingStyle::wrappingStyleForSerialization(*m_highestNodeToBeSerialized->parentNode(), shouldAnnotate());
     584        m_wrappingStyle = EditingStyle::wrappingStyleForSerialization(*m_highestNodeToBeSerialized->parentNode(), shouldAnnotate(), m_standardFontFamilySerializationMode);
    583585
    584586    return traverseNodesForSerialization(startNode.get(), pastEnd, NodeTraversalMode::EmitString);
    … …  
    823825
    824826static String serializePreservingVisualAppearanceInternal(const Position& start, const Position& end, Vector<Node*>* nodes, ResolveURLs urlsToResolve, SerializeComposedTree serializeComposedTree,
    825     AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, MSOListMode msoListMode)
     827    AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, StandardFontFamilySerializationMode standardFontFamilySerializationMode, MSOListMode msoListMode)
    826828{
    827829    static NeverDestroyed<const String> interchangeNewlineString(MAKE_STATIC_STRING_IMPL("<br class=\"" AppleInterchangeNewline "\">"));
    … …  
    849851    Node* specialCommonAncestor = highestAncestorToWrapMarkup(start, end, *commonAncestor, annotate);
    850852
    851     StyledMarkupAccumulator accumulator(start, end, nodes, urlsToResolve, serializeComposedTree, annotate, msoListMode, needsPositionStyleConversion, specialCommonAncestor);
     853    StyledMarkupAccumulator accumulator(start, end, nodes, urlsToResolve, serializeComposedTree, annotate, standardFontFamilySerializationMode, msoListMode, needsPositionStyleConversion, specialCommonAncestor);
    852854
    853855    Position startAdjustedForInterchangeNewline = start;
    … …  
    918920{
    919921    return serializePreservingVisualAppearanceInternal(range.startPosition(), range.endPosition(), nodes, urlsToReslve, SerializeComposedTree::No,
    920         annotate, convertBlocksToInlines, MSOListMode::DoNotPreserve);
     922        annotate, convertBlocksToInlines, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve);
    921923}
    922924
    … …  
    924926{
    925927    return serializePreservingVisualAppearanceInternal(selection.start(), selection.end(), nodes, resolveURLs, serializeComposedTree,
    926         AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, MSOListMode::DoNotPreserve);
     928        AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve);
    927929}
    928930
    … …  
    951953    // SerializeComposedTree::No because there can't be a shadow tree in the pasted fragment.
    952954    auto result = serializePreservingVisualAppearanceInternal(firstPositionInNode(bodyElement.get()), lastPositionInNode(bodyElement.get()), nullptr,
    953         ResolveURLs::YesExcludingLocalFileURLsForPrivacy, SerializeComposedTree::No, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No,  msoListMode);
     955        ResolveURLs::YesExcludingLocalFileURLsForPrivacy, SerializeComposedTree::No, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No,  StandardFontFamilySerializationMode::Strip, msoListMode);
    954956
    955957    if (msoListMode == MSOListMode::Preserve) {

  • branches/safari-608-branch/Tools/ChangeLog

    r249698 r249707  
     12019-09-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (iOS 13): Bulleted list copied from Notes to Mail results in Times New Roman
     4        https://bugs.webkit.org/show_bug.cgi?id=201490
     5
     6        Reviewed by Daniel Bates.
     7
     8        Added a test.
     9
     10        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     11        * TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:
     12        * TestWebKitAPI/Tests/WebKitCocoa/cocoa-writer-markup-with-lists.html: Added.
     13
    1142019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    215

  • branches/safari-608-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r249697 r249707  
    715715                9B7A37C41F8AEBA5004AA228 /* CopyURL.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9B7A37C21F8AEBA5004AA228 /* CopyURL.mm */; };
    716716                9B7D740F1F8378770006C432 /* paste-rtfd.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B7D740E1F8377E60006C432 /* paste-rtfd.html */; };
     717                9B9332CE2320C745002D50E8 /* cocoa-writer-markup-with-lists.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B9332CD2320C73E002D50E8 /* cocoa-writer-markup-with-lists.html */; };
    717718                9BAD7F3E22690F2000F8DA66 /* DeallocWebViewInEventListener.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9BAD7F3D22690F1400F8DA66 /* DeallocWebViewInEventListener.mm */; };
    718719                9BAE177B22E2BBFB00DF3098 /* cocoa-writer-markup-with-system-fonts.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9BAE177A22E2BB6B00DF3098 /* cocoa-writer-markup-with-system-fonts.html */; };
    … …  
    11421143                                9B270FEE1DDC2C0B002D53F3 /* closed-shadow-tree-test.html in Copy Resources */,
    11431144                                E5036F78211BC25400BFDBE2 /* color-drop.html in Copy Resources */,
     1145                                9B9332CE2320C745002D50E8 /* cocoa-writer-markup-with-lists.html in Copy Resources */,
    11441146                                F4B825D81EF4DBFB006E417F /* compressed-files.zip in Copy Resources */,
    11451147                                5C9E56871DF914AE00C9EE33 /* contentBlockerCheck.html in Copy Resources */,
    … …  
    20612063                9B7A37C21F8AEBA5004AA228 /* CopyURL.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CopyURL.mm; sourceTree = "<group>"; };
    20622064                9B7D740E1F8377E60006C432 /* paste-rtfd.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "paste-rtfd.html"; sourceTree = "<group>"; };
     2065                9B9332CD2320C73E002D50E8 /* cocoa-writer-markup-with-lists.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "cocoa-writer-markup-with-lists.html"; sourceTree = "<group>"; };
    20632066                9BAD7F3D22690F1400F8DA66 /* DeallocWebViewInEventListener.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeallocWebViewInEventListener.mm; sourceTree = "<group>"; };
    20642067                9BAE177A22E2BB6B00DF3098 /* cocoa-writer-markup-with-system-fonts.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "cocoa-writer-markup-with-system-fonts.html"; sourceTree = "<group>"; };
    … …  
    31373140                                F4B825D61EF4DBD4006E417F /* compressed-files.zip */,
    31383141                                F469FB231F01803500401539 /* contenteditable-and-target.html */,
     3142                                9B9332CD2320C73E002D50E8 /* cocoa-writer-markup-with-lists.html */,
    31393143                                F41AB99C1EF4692C0083FA08 /* contenteditable-and-textarea.html */,
    31403144                                F4A32ECA1F0642F40047C544 /* contenteditable-in-iframe.html */,

  • branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm

    r247966 r249707  
    359359
    360360#endif // PLATFORM(COCOA)
     361TEST(PasteHTML, DoesNotAddStandardFontFamily)
     362{
     363    writeHTMLToPasteboard([NSString stringWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"cocoa-writer-markup-with-lists" ofType:@"html" inDirectory:@"TestWebKitAPI.resources"] encoding:NSUTF8StringEncoding error:NULL]);
     364
     365    auto webView = createWebViewWithCustomPasteboardDataSetting(true);
     366    [webView synchronouslyLoadTestPageNamed:@"paste-rtfd"];
     367    [webView stringByEvaluatingJavaScript:@"document.body.style.fontFamily = 'Arial'"];
     368    [webView paste:nil];
     369
     370    EXPECT_WK_STREQ("[\"text/html\"]", [webView stringByEvaluatingJavaScript:@"JSON.stringify(clipboardData.types)"]);
     371    [webView stringByEvaluatingJavaScript:@"window.htmlInDataTransfer = clipboardData.values[0]"];
     372    [webView stringByEvaluatingJavaScript:@"window.pastedHTML = editor.innerHTML"];
     373
     374    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"pastedHTML.includes('Hello')"].boolValue);
     375    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"pastedHTML.includes('font-weight: bold')"].boolValue);
     376    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!pastedHTML.includes('-webkit-standard')"].boolValue);
     377
     378    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"htmlInDataTransfer.includes('Hello')"].boolValue);
     379    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"htmlInDataTransfer.includes('font-weight: bold')"].boolValue);
     380    EXPECT_TRUE([webView stringByEvaluatingJavaScript:@"!htmlInDataTransfer.includes('-webkit-standard')"].boolValue);
     381
     382    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"getComputedStyle(document.querySelector('.s2')).fontFamily"],
     383        [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.body).fontFamily"]);
     384    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"getComputedStyle(document.querySelector('.s4')).fontFamily"],
     385        [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.body).fontFamily"]);
     386}
     387
Note: See TracChangeset for help on using the changeset viewer.