Changeset 236854 in webkit


Ignore:
Timestamp:
Oct 4, 2018 3:08:51 PM (6 years ago)
Author:
Wenson Hsieh
Message:

[macOS] Fix some font attribute conversion bugs in preparation for "Font > Styles…" support in WebKit2
​https://bugs.webkit.org/show_bug.cgi?id=190289
<rdar://problem/45020806>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Makes some small adjustments to fix two bugs in font attribute conversion logic. See below for more detail.

Tests: FontManagerTests.AddFontShadowUsingFontOptions

FontManagerTests.AddAndRemoveColorsUsingFontOptions

  • editing/FontAttributeChanges.cpp:

(WebCore::cssValueListForShadow):

  • editing/cocoa/FontAttributesCocoa.mm:

Currently, we bail from adding a font shadow if the shadow's offset is empty. However, valid shadow offsets may
have negative dimensions, so a check for isZero() should be used instead.

(WebCore::FontAttributes::createDictionary const):

  • platform/mac/WebCoreNSFontManagerExtras.mm:

Fall back to a transparent background color; this allows senders to remove the current background color by just
removing NSBackgroundColorAttributeName from the attribute dictionary, rather than explicitly setting it to the
transparent color (this scenario is exercised when using "Font > Styles…" to specify a font style without a
background color).

(WebCore::computedFontAttributeChanges):

Tools:

Add new API tests to exercise two corner cases when using NSFontOptions ("Font > Styles…") to change font
attributes at the current selection.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h:
  • TestWebKitAPI/Tests/mac/FontManagerTests.mm:

(webViewForFontManagerTesting):
(TestWebKitAPI::TEST):

  • TestWebKitAPI/mac/TestFontOptions.h: Copied from Source/WebCore/editing/cocoa/FontAttributesCocoa.mm.
  • TestWebKitAPI/mac/TestFontOptions.mm: Added.

Introduce TestFontOptions, which wraps the shared NSFontOptions and swizzles -sharedFontOptions to return a
global instance of itself. TestFontOptions supports several testing helpers to add or remove font shadows,
foreground colors, and background colors.

(sharedFontOptionsForTesting):
(+[TestFontOptions sharedInstance]):
(-[TestFontOptions initWithFontOptions:]):
(-[TestFontOptions selectedAttributes]):
(-[TestFontOptions fontOptions]):
(-[TestFontOptions shadowWidth]):
(-[TestFontOptions setShadowWidth:]):
(-[TestFontOptions shadowHeight]):
(-[TestFontOptions setShadowHeight:]):
(-[TestFontOptions setShadowBlurRadius:]):
(-[TestFontOptions setHasShadow:]):
(-[TestFontOptions foregroundColor]):
(-[TestFontOptions setForegroundColor:]):
(-[TestFontOptions backgroundColor]):
(-[TestFontOptions setBackgroundColor:]):
(-[TestFontOptions _dispatchFontAttributeChanges]):
(-[TestFontOptions convertAttributes:]):
(-[TestFontOptions setSelectedAttributes:isMultiple:]):
(-[TestFontOptions forwardInvocation:]):

Location:
trunk
Files:
1 added
8 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r236850 r236854  
     12018-10-04  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS] Fix some font attribute conversion bugs in preparation for "Font > Styles…" support in WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=190289
     5        <rdar://problem/45020806>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Makes some small adjustments to fix two bugs in font attribute conversion logic. See below for more detail.
     10
     11        Tests:  FontManagerTests.AddFontShadowUsingFontOptions
     12                FontManagerTests.AddAndRemoveColorsUsingFontOptions
     13
     14        * editing/FontAttributeChanges.cpp:
     15        (WebCore::cssValueListForShadow):
     16        * editing/cocoa/FontAttributesCocoa.mm:
     17
     18        Currently, we bail from adding a font shadow if the shadow's offset is empty. However, valid shadow offsets may
     19        have negative dimensions, so a check for `isZero()` should be used instead.
     20
     21        (WebCore::FontAttributes::createDictionary const):
     22        * platform/mac/WebCoreNSFontManagerExtras.mm:
     23
     24        Fall back to a transparent background color; this allows senders to remove the current background color by just
     25        removing NSBackgroundColorAttributeName from the attribute dictionary, rather than explicitly setting it to the
     26        transparent color (this scenario is exercised when using "Font > Styles…" to specify a font style without a
     27        background color).
     28
     29        (WebCore::computedFontAttributeChanges):
     30
    1312018-10-03  Ryosuke Niwa  <rniwa@webkit.org>
    232

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

    r236445 r236854  
    8282static RefPtr<CSSValueList> cssValueListForShadow(const FontShadow& shadow)
    8383{
    84     if (shadow.offset.isEmpty() && !shadow.blurRadius)
     84    if (shadow.offset.isZero() && !shadow.blurRadius)
    8585        return nullptr;
    8686

  • trunk/Source/WebCore/editing/cocoa/FontAttributesCocoa.mm

    r236468 r236854  
    4343        attributes[NSBackgroundColorAttributeName] = platformColor(backgroundColor);
    4444
    45     if (fontShadow.color.isValid() && (!fontShadow.offset.isEmpty() || fontShadow.blurRadius)) {
    46         auto shadow = fontShadow.createShadow();
    47         attributes[NSShadowAttributeName] = shadow.get();
    48     }
     45    if (fontShadow.color.isValid() && (!fontShadow.offset.isZero() || fontShadow.blurRadius))
     46        attributes[NSShadowAttributeName] = fontShadow.createShadow().get();
    4947
    5048    if (subscriptOrSuperscript == SubscriptOrSuperscript::Subscript)

  • trunk/Source/WebCore/platform/mac/WebCoreNSFontManagerExtras.mm

    r236445 r236854  
    127127    NSColor *convertedBackgroundColorA = [convertedAttributesA objectForKey:NSBackgroundColorAttributeName];
    128128    if (convertedBackgroundColorA == [convertedAttributesB objectForKey:NSBackgroundColorAttributeName])
    129         changes.setBackgroundColor(colorFromNSColor(convertedBackgroundColorA));
     129        changes.setBackgroundColor(colorFromNSColor(convertedBackgroundColorA ?: NSColor.clearColor));
    130130
    131131    changes.setFontChanges(computedFontChanges(fontManager, originalFontA, [convertedAttributesA objectForKey:NSFontAttributeName], [convertedAttributesB objectForKey:NSFontAttributeName]));

  • trunk/Tools/ChangeLog

    r236842 r236854  
     12018-10-04  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS] Fix some font attribute conversion bugs in preparation for "Font > Styles…" support in WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=190289
     5        <rdar://problem/45020806>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Add new API tests to exercise two corner cases when using NSFontOptions ("Font > Styles…") to change font
     10        attributes at the current selection.
     11
     12        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     13        * TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h:
     14        * TestWebKitAPI/Tests/mac/FontManagerTests.mm:
     15        (webViewForFontManagerTesting):
     16        (TestWebKitAPI::TEST):
     17        * TestWebKitAPI/mac/TestFontOptions.h: Copied from Source/WebCore/editing/cocoa/FontAttributesCocoa.mm.
     18        * TestWebKitAPI/mac/TestFontOptions.mm: Added.
     19
     20        Introduce TestFontOptions, which wraps the shared NSFontOptions and swizzles `-sharedFontOptions` to return a
     21        global instance of itself. TestFontOptions supports several testing helpers to add or remove font shadows,
     22        foreground colors, and background colors.
     23
     24        (sharedFontOptionsForTesting):
     25        (+[TestFontOptions sharedInstance]):
     26        (-[TestFontOptions initWithFontOptions:]):
     27        (-[TestFontOptions selectedAttributes]):
     28        (-[TestFontOptions fontOptions]):
     29        (-[TestFontOptions shadowWidth]):
     30        (-[TestFontOptions setShadowWidth:]):
     31        (-[TestFontOptions shadowHeight]):
     32        (-[TestFontOptions setShadowHeight:]):
     33        (-[TestFontOptions setShadowBlurRadius:]):
     34        (-[TestFontOptions setHasShadow:]):
     35        (-[TestFontOptions foregroundColor]):
     36        (-[TestFontOptions setForegroundColor:]):
     37        (-[TestFontOptions backgroundColor]):
     38        (-[TestFontOptions setBackgroundColor:]):
     39        (-[TestFontOptions _dispatchFontAttributeChanges]):
     40        (-[TestFontOptions convertAttributes:]):
     41        (-[TestFontOptions setSelectedAttributes:isMultiple:]):
     42        (-[TestFontOptions forwardInvocation:]):
     43
    1442018-10-04  Jiewen Tan  <jiewen_tan@apple.com>
    245

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

    r236724 r236854  
    857857                F4F405BC1D4C0D1C007A9707 /* full-size-autoplaying-video-with-audio.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4F405BA1D4C0CF8007A9707 /* full-size-autoplaying-video-with-audio.html */; };
    858858                F4F405BD1D4C0D1C007A9707 /* skinny-autoplaying-video-with-audio.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4F405BB1D4C0CF8007A9707 /* skinny-autoplaying-video-with-audio.html */; };
     859                F4F5BB5221667BAA002D06B9 /* TestFontOptions.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4F5BB5121667BAA002D06B9 /* TestFontOptions.mm */; };
    859860                F4FA91811E61849B007B8C1D /* WKWebViewMacEditingTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4FA917F1E61849B007B8C1D /* WKWebViewMacEditingTests.mm */; };
    860861                F4FA91831E61857B007B8C1D /* double-click-does-not-select-trailing-space.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4FA91821E618566007B8C1D /* double-click-does-not-select-trailing-space.html */; };
    … …  
    21152116                F4F405BA1D4C0CF8007A9707 /* full-size-autoplaying-video-with-audio.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "full-size-autoplaying-video-with-audio.html"; sourceTree = "<group>"; };
    21162117                F4F405BB1D4C0CF8007A9707 /* skinny-autoplaying-video-with-audio.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "skinny-autoplaying-video-with-audio.html"; sourceTree = "<group>"; };
     2118                F4F5BB5021667BAA002D06B9 /* TestFontOptions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestFontOptions.h; sourceTree = "<group>"; };
     2119                F4F5BB5121667BAA002D06B9 /* TestFontOptions.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TestFontOptions.mm; sourceTree = "<group>"; };
    21172120                F4FA917F1E61849B007B8C1D /* WKWebViewMacEditingTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewMacEditingTests.mm; sourceTree = "<group>"; };
    21182121                F4FA91821E618566007B8C1D /* double-click-does-not-select-trailing-space.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = "double-click-does-not-select-trailing-space.html"; path = "Tests/WebKitCocoa/double-click-does-not-select-trailing-space.html"; sourceTree = SOURCE_ROOT; };
    … …  
    31753178                                F4E0A2B62122847400AF7C7F /* TestFilePromiseReceiver.h */,
    31763179                                F4E0A2B72122847400AF7C7F /* TestFilePromiseReceiver.mm */,
     3180                                F4F5BB5021667BAA002D06B9 /* TestFontOptions.h */,
     3181                                F4F5BB5121667BAA002D06B9 /* TestFontOptions.mm */,
    31773182                                F45D388F215A7B4B002A2979 /* TestInspectorBar.h */,
    31783183                                F45D3890215A7B4B002A2979 /* TestInspectorBar.mm */,
    … …  
    39964001                                F46128CB211D475100D9FADB /* TestDraggingInfo.mm in Sources */,
    39974002                                F4E0A2B82122847400AF7C7F /* TestFilePromiseReceiver.mm in Sources */,
     4003                                F4F5BB5221667BAA002D06B9 /* TestFontOptions.mm in Sources */,
    39984004                                F45E15762112CE6200307E82 /* TestInputDelegate.mm in Sources */,
    39994005                                F45D3891215A7B4B002A2979 /* TestInspectorBar.mm in Sources */,

  • trunk/Tools/TestWebKitAPI/Tests/TestWebKitAPI/mac/AppKitSPI.h

    r236724 r236854  
    104104@end
    105105
     106@interface NSFontOptions : NSObject
     107+ (instancetype)sharedFontOptions;
     108@end
     109
    106110#endif // PLATFORM(MAC)

  • trunk/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm

    r236770 r236854  
    3232#import "NSFontPanelTesting.h"
    3333#import "PlatformUtilities.h"
     34#import "TestFontOptions.h"
    3435#import "TestInspectorBar.h"
    3536#import "TestWKWebView.h"
    … …  
    114115@end
    115116
    116 static RetainPtr<FontManagerTestWKWebView> webViewForFontManagerTesting(NSFontManager *fontManager)
     117static RetainPtr<FontManagerTestWKWebView> webViewForFontManagerTesting(NSFontManager *fontManager, NSString *markup)
    117118{
    118119    auto webView = adoptNS([[FontManagerTestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
    119     [webView synchronouslyLoadHTMLString:@"<body contenteditable>"
    120         "<span id='foo'>foo</span> <span id='bar'>bar</span> <span id='baz'>baz</span>"
    121         "</body><script>document.body.addEventListener('input', event => lastInputEvent = event)</script>"];
     120    [webView synchronouslyLoadHTMLString:markup];
    122121    [webView stringByEvaluatingJavaScript:@"document.body.focus()"];
    123122    [webView _setEditable:YES];
    124123    fontManager.target = webView.get();
    125124    return webView;
     125}
     126
     127static RetainPtr<FontManagerTestWKWebView> webViewForFontManagerTesting(NSFontManager *fontManager)
     128{
     129    return webViewForFontManagerTesting(fontManager, @"<body contenteditable>"
     130        "<span id='foo'>foo</span> <span id='bar'>bar</span> <span id='baz'>baz</span>"
     131        "</body><script>document.body.addEventListener('input', event => lastInputEvent = event)</script>");
    126132}
    127133
    … …  
    475481}
    476482
     483TEST(FontManagerTests, AddFontShadowUsingFontOptions)
     484{
     485    TestFontOptions *options = TestFontOptions.sharedInstance;
     486    auto webView = webViewForFontManagerTesting(NSFontManager.sharedFontManager);
     487
     488    [webView selectWord:nil];
     489    options.shadowWidth = 3;
     490    options.shadowHeight = -3;
     491    options.hasShadow = YES;
     492
     493    EXPECT_WK_STREQ("rgba(0, 0, 0, 0.333333) 3px -3px 0px", [webView stylePropertyAtSelectionStart:@"text-shadow"]);
     494    [webView waitForNextPresentationUpdate];
     495    NSShadow *shadow = [webView typingAttributes][NSShadowAttributeName];
     496    EXPECT_EQ(shadow.shadowOffset.width, 3);
     497    EXPECT_EQ(shadow.shadowOffset.height, -3);
     498}
     499
     500TEST(FontManagerTests, AddAndRemoveColorsUsingFontOptions)
     501{
     502    TestFontOptions *options = TestFontOptions.sharedInstance;
     503    auto webView = webViewForFontManagerTesting(NSFontManager.sharedFontManager, @"<body contenteditable>hello</body>");
     504    [webView selectWord:nil];
     505
     506    options.backgroundColor = [NSColor colorWithRed:1 green:0 blue:0 alpha:0.2];
     507    options.foregroundColor = [NSColor colorWithRed:0 green:0 blue:1 alpha:1];
     508
     509    EXPECT_WK_STREQ("rgb(0, 0, 255)", [webView stylePropertyAtSelectionStart:@"color"]);
     510    EXPECT_WK_STREQ("rgba(255, 0, 0, 0.2)", [webView stylePropertyAtSelectionStart:@"background-color"]);
     511    NSDictionary *attributes = [webView typingAttributes];
     512    EXPECT_TRUE([[NSColor colorWithRed:0 green:0 blue:1 alpha:1] isEqual:attributes[NSForegroundColorAttributeName]]);
     513    EXPECT_TRUE([[NSColor colorWithRed:1 green:0 blue:0 alpha:0.2] isEqual:attributes[NSBackgroundColorAttributeName]]);
     514
     515    options.backgroundColor = nil;
     516    options.foregroundColor = nil;
     517
     518    EXPECT_WK_STREQ("rgb(0, 0, 0)", [webView stylePropertyAtSelectionStart:@"color"]);
     519    EXPECT_WK_STREQ("rgba(0, 0, 0, 0)", [webView stylePropertyAtSelectionStart:@"background-color"]);
     520    attributes = [webView typingAttributes];
     521    EXPECT_NULL(attributes[NSForegroundColorAttributeName]);
     522    EXPECT_NULL(attributes[NSBackgroundColorAttributeName]);
     523}
     524
    477525} // namespace TestWebKitAPI
    478526

  • trunk/Tools/TestWebKitAPI/mac/TestFontOptions.h

    r236853 r236854  
    2424 */
    2525
    26 #import "config.h"
    27 #import "FontAttributes.h"
     26#pragma once
    2827
    29 #import "ColorCocoa.h"
     28#if PLATFORM(MAC) && WK_API_ENABLED
    3029
    31 namespace WebCore {
     30#import <AppKit/AppKit.h>
    3231
    33 RetainPtr<NSDictionary> FontAttributes::createDictionary() const
    34 {
    35     NSMutableDictionary *attributes = [NSMutableDictionary dictionary];
    36     if (font)
    37         attributes[NSFontAttributeName] = font.get();
     32@interface TestFontOptions : NSObject
    3833
    39     if (foregroundColor.isValid())
    40         attributes[NSForegroundColorAttributeName] = platformColor(foregroundColor);
     34@property (class, readonly) TestFontOptions *sharedInstance;
     35@property (nonatomic, readonly) NSDictionary *selectedAttributes;
     36@property (nonatomic, readonly) BOOL hasMultipleFonts;
    4137
    42     if (backgroundColor.isValid())
    43         attributes[NSBackgroundColorAttributeName] = platformColor(backgroundColor);
     38// Font shadow manipulation.
     39@property (nonatomic) BOOL hasShadow;
     40@property (nonatomic) CGFloat shadowWidth;
     41@property (nonatomic) CGFloat shadowHeight;
     42@property (nonatomic) CGFloat shadowBlurRadius;
    4443
    45     if (fontShadow.color.isValid() && (!fontShadow.offset.isEmpty() || fontShadow.blurRadius)) {
    46         auto shadow = fontShadow.createShadow();
    47         attributes[NSShadowAttributeName] = shadow.get();
    48     }
     44// Font colors.
     45@property (nonatomic, copy) NSColor *foregroundColor;
     46@property (nonatomic, copy) NSColor *backgroundColor;
    4947
    50     if (subscriptOrSuperscript == SubscriptOrSuperscript::Subscript)
    51         attributes[NSSuperscriptAttributeName] = @(-1);
    52     else if (subscriptOrSuperscript == SubscriptOrSuperscript::Superscript)
    53         attributes[NSSuperscriptAttributeName] = @1;
     48@end
    5449
    55     if (hasUnderline)
    56         attributes[NSUnderlineStyleAttributeName] = @(NSUnderlineStyleSingle);
    57 
    58     if (hasStrikeThrough)
    59         attributes[NSStrikethroughStyleAttributeName] = @(NSUnderlineStyleSingle);
    60 
    61     return attributes;
    62 }
    63 
    64 } // namespace WebCore
     50#endif
Note: See TracChangeset for help on using the changeset viewer.