Changeset 247439 in webkit


Ignore:
Timestamp:
Jul 15, 2019 12:09:51 PM (5 years ago)
Author:
Wenson Hsieh
Message:

[macOS 10.15] Cannot unbold selected text when the system font is used
https://bugs.webkit.org/show_bug.cgi?id=199788
<rdar://problem/52142570>

Reviewed by Tim Horton.

Source/WebKit:

In macOS 10.15, +[NSFont fontWithName:size:] no longer recognizes system fonts (of name
".SFNS-*") and returns nil instead. However, our existing implementation of
WebPageProxy::fontAtSelection works by grabbing the font name in the web process, and
sending it over to the UI process, where it is mapped to an NSFont. As a result, this always
results in a nil font in macOS 10.15, which causes us to never update NSFontManager's
selected font. In turn, this means that once selected text is bolded, it can't be unbolded
via NSFontManager, since NSFontManager thinks that the text is still not bold.

To fix this, we simply encode and send a platform FontInfo instead of sending the font name.
This allows the UI process to reconstruct NSFonts from font attribute dictionaries instead,
and update the font manager.

  • UIProcess/Cocoa/WebViewImpl.mm:

(WebKit::WebViewImpl::updateFontManagerIfNeeded):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • UIProcess/mac/WebPageProxyMac.mm:

(WebKit::WebPageProxy::fontAtSelection):

Refactor this to send a FontInfo (containing a font attribute dictionary) instead of a font
name.

(WebKit::WebPageProxy::fontAtSelectionCallback): Deleted.

  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/WebPage.messages.in:

Change FontAtSelection to use sendWithAsyncReply instead of sending a callback ID. This also
allows us to remove FontAtSelectionCallback.

  • WebProcess/WebPage/mac/WebPageMac.mm:

(WebKit::WebPage::fontAtSelection):

Tools:

Add a new API test to verify that bolding and unbolding updates the
shared font manager's selected font.

  • TestWebKitAPI/Tests/mac/FontManagerTests.mm:

(TestWebKitAPI::TEST):

Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r247437 r247439  
     12019-07-15  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS 10.15] Cannot unbold selected text when the system font is used
     4        https://bugs.webkit.org/show_bug.cgi?id=199788
     5        <rdar://problem/52142570>
     6
     7        Reviewed by Tim Horton.
     8
     9        In macOS 10.15, +[NSFont fontWithName:size:] no longer recognizes system fonts (of name
     10        ".SFNS-*") and returns nil instead. However, our existing implementation of
     11        WebPageProxy::fontAtSelection works by grabbing the font name in the web process, and
     12        sending it over to the UI process, where it is mapped to an NSFont. As a result, this always
     13        results in a nil font in macOS 10.15, which causes us to never update NSFontManager's
     14        selected font. In turn, this means that once selected text is bolded, it can't be unbolded
     15        via NSFontManager, since NSFontManager thinks that the text is still not bold.
     16
     17        To fix this, we simply encode and send a platform FontInfo instead of sending the font name.
     18        This allows the UI process to reconstruct NSFonts from font attribute dictionaries instead,
     19        and update the font manager.
     20
     21        * UIProcess/Cocoa/WebViewImpl.mm:
     22        (WebKit::WebViewImpl::updateFontManagerIfNeeded):
     23        * UIProcess/WebPageProxy.h:
     24        * UIProcess/WebPageProxy.messages.in:
     25        * UIProcess/mac/WebPageProxyMac.mm:
     26        (WebKit::WebPageProxy::fontAtSelection):
     27
     28        Refactor this to send a FontInfo (containing a font attribute dictionary) instead of a font
     29        name.
     30
     31        (WebKit::WebPageProxy::fontAtSelectionCallback): Deleted.
     32        * WebProcess/WebPage/WebPage.h:
     33        * WebProcess/WebPage/WebPage.messages.in:
     34
     35        Change FontAtSelection to use sendWithAsyncReply instead of sending a callback ID. This also
     36        allows us to remove FontAtSelectionCallback.
     37
     38        * WebProcess/WebPage/mac/WebPageMac.mm:
     39        (WebKit::WebPage::fontAtSelection):
     40
    1412019-07-15  Jiewen Tan  <jiewen_tan@apple.com>
    242
  • trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

    r247120 r247439  
    3535#import "AttributedString.h"
    3636#import "ColorSpaceData.h"
     37#import "FontInfo.h"
    3738#import "FullscreenClient.h"
    3839#import "GenericCallback.h"
     
    28102811        return;
    28112812
    2812     m_page->fontAtSelection([](const String& fontName, double fontSize, bool selectionHasMultipleFonts, WebKit::CallbackBase::Error error) {
    2813         if (NSFont *font = [NSFont fontWithName:fontName size:fontSize])
    2814             [NSFontManager.sharedFontManager setSelectedFont:font isMultiple:selectionHasMultipleFonts];
     2813    m_page->fontAtSelection([](const FontInfo& fontInfo, double fontSize, bool selectionHasMultipleFonts) {
     2814        NSDictionary *attributeDictionary = (__bridge NSDictionary *)fontInfo.fontAttributeDictionary.get();
     2815        if (!attributeDictionary)
     2816            return;
     2817
     2818        NSFontDescriptor *descriptor = [NSFontDescriptor fontDescriptorWithFontAttributes:attributeDictionary];
     2819        if (!descriptor)
     2820            return;
     2821
     2822        NSFont *font = [NSFont fontWithDescriptor:descriptor size:fontSize];
     2823        if (!font)
     2824            return;
     2825
     2826        [NSFontManager.sharedFontManager setSelectedFont:font isMultiple:selectionHasMultipleFonts];
    28152827    });
    28162828}
  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r247344 r247439  
    283283struct EditingRange;
    284284struct EditorState;
     285struct FontInfo;
    285286struct FrameInfoData;
    286287struct InsertTextOptions;
     
    338339#if PLATFORM(MAC)
    339340typedef GenericCallback<const AttributedString&, const EditingRange&> AttributedStringForCharacterRangeCallback;
    340 typedef GenericCallback<const String&, double, bool> FontAtSelectionCallback;
    341341#endif
    342342
     
    789789    void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::TextAlternativeWithRange>& dictationAlternatives, bool registerUndoGroup);
    790790    void attributedSubstringForCharacterRangeAsync(const EditingRange&, WTF::Function<void (const AttributedString&, const EditingRange&, CallbackBase::Error)>&&);
    791     void fontAtSelection(WTF::Function<void (const String&, double, bool, CallbackBase::Error)>&&);
     791    void fontAtSelection(Function<void(const FontInfo&, double, bool)>&&);
    792792
    793793    void startWindowDrag();
     
    18701870#if PLATFORM(MAC)
    18711871    void attributedStringForCharacterRangeCallback(const AttributedString&, const EditingRange&, CallbackID);
    1872     void fontAtSelectionCallback(const String&, double, bool, CallbackID);
    18731872#endif
    18741873#if PLATFORM(IOS_FAMILY)
  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r246660 r247439  
    181181#if PLATFORM(MAC)
    182182    AttributedStringForCharacterRangeCallback(struct WebKit::AttributedString string, struct WebKit::EditingRange actualRange, WebKit::CallbackID callbackID)
    183     FontAtSelectionCallback(String fontName, double fontSize, bool selectioHasMultipleFonts, WebKit::CallbackID callbackID)
    184183#endif
    185184    FontAttributesCallback(struct WebCore::FontAttributes attributes, WebKit::CallbackID callbackID)
  • trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm

    r245073 r247439  
    3434#import "DataReference.h"
    3535#import "EditorState.h"
     36#import "FontInfo.h"
    3637#import "InsertTextOptions.h"
    3738#import "MenuUtilities.h"
     
    241242}
    242243
    243 void WebPageProxy::fontAtSelection(WTF::Function<void (const String&, double, bool, CallbackBase::Error)>&& callbackFunction)
     244void WebPageProxy::fontAtSelection(Function<void(const FontInfo&, double, bool)>&& callback)
    244245{
    245246    if (!hasRunningProcess()) {
    246         callbackFunction(String(), 0, false, CallbackBase::Error::Unknown);
    247         return;
    248     }
    249    
    250     auto callbackID = m_callbacks.put(WTFMove(callbackFunction), m_process->throttler().backgroundActivityToken());
    251    
    252     process().send(Messages::WebPage::FontAtSelection(callbackID), m_pageID);
    253 }
    254 
    255 void WebPageProxy::fontAtSelectionCallback(const String& fontName, double fontSize, bool selectionHasMultipleFonts, CallbackID callbackID)
    256 {
    257     auto callback = m_callbacks.take<FontAtSelectionCallback>(callbackID);
    258     if (!callback) {
    259         // FIXME: Log error or assert.
    260         // this can validly happen if a load invalidated the callback, though
    261         return;
    262     }
    263    
    264     callback->performCallbackWithReturnValue(fontName, fontSize, selectionHasMultipleFonts);
     247        callback({ }, 0, false);
     248        return;
     249    }
     250    m_process->connection()->sendWithAsyncReply(Messages::WebPage::FontAtSelection(), WTFMove(callback), m_pageID);
    265251}
    266252
  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r247398 r247439  
    259259struct DataDetectionResult;
    260260struct EditorState;
     261struct FontInfo;
    261262struct InsertTextOptions;
    262263struct InteractionInformationAtPosition;
     
    811812    void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::DictationAlternative>& dictationAlternativeLocations, bool registerUndoGroup = false);
    812813    void attributedSubstringForCharacterRangeAsync(const EditingRange&, CallbackID);
    813     void fontAtSelection(CallbackID);
     814    void fontAtSelection(CompletionHandler<void(const FontInfo&, double, bool)>&&);
    814815#endif
    815816
  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

    r247161 r247439  
    446446    InsertDictatedTextAsync(String text, struct WebKit::EditingRange replacementRange, Vector<WebCore::DictationAlternative> dictationAlternatives, bool registerUndoGroup)
    447447    AttributedSubstringForCharacterRangeAsync(struct WebKit::EditingRange range, WebKit::CallbackID callbackID);
    448     FontAtSelection(WebKit::CallbackID callbackID);
     448    FontAtSelection() -> (struct WebKit::FontInfo info, double fontSize, bool hasMultipleFonts) Async
    449449#endif
    450450
  • trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

    r247416 r247439  
    3434#import "EditingRange.h"
    3535#import "EditorState.h"
     36#import "FontInfo.h"
    3637#import "InjectedBundleHitTestResult.h"
    3738#import "PDFKitImports.h"
     
    377378}
    378379
    379 void WebPage::fontAtSelection(CallbackID callbackID)
    380 {
    381     String fontName;
    382     double fontSize = 0;
     380void WebPage::fontAtSelection(CompletionHandler<void(const FontInfo&, double, bool)>&& reply)
     381{
    383382    bool selectionHasMultipleFonts = false;
    384     Frame& frame = m_page->focusController().focusedOrMainFrame();
    385    
    386     if (!frame.selection().selection().isNone()) {
    387         if (auto* font = frame.editor().fontForSelection(selectionHasMultipleFonts)) {
    388             if (auto ctFont = font->getCTFont()) {
    389                 fontName = adoptCF(CTFontCopyPostScriptName(ctFont)).get();
    390                 fontSize = CTFontGetSize(ctFont);
    391             }
    392         }
    393     }
    394     send(Messages::WebPageProxy::FontAtSelectionCallback(fontName, fontSize, selectionHasMultipleFonts, callbackID));
     383    auto& frame = m_page->focusController().focusedOrMainFrame();
     384
     385    if (frame.selection().selection().isNone()) {
     386        reply({ }, 0, false);
     387        return;
     388    }
     389
     390    auto* font = frame.editor().fontForSelection(selectionHasMultipleFonts);
     391    if (!font) {
     392        reply({ }, 0, false);
     393        return;
     394    }
     395
     396    auto ctFont = font->getCTFont();
     397    if (!ctFont) {
     398        reply({ }, 0, false);
     399        return;
     400    }
     401
     402    auto fontDescriptor = adoptCF(CTFontCopyFontDescriptor(ctFont));
     403    if (!fontDescriptor) {
     404        reply({ }, 0, false);
     405        return;
     406    }
     407
     408    reply({ adoptCF(CTFontDescriptorCopyAttributes(fontDescriptor.get())) }, CTFontGetSize(ctFont), selectionHasMultipleFonts);
    395409}
    396410   
  • trunk/Tools/ChangeLog

    r247437 r247439  
     12019-07-15  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [macOS 10.15] Cannot unbold selected text when the system font is used
     4        https://bugs.webkit.org/show_bug.cgi?id=199788
     5        <rdar://problem/52142570>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a new API test to verify that bolding and unbolding updates the
     10        shared font manager's selected font.
     11
     12        * TestWebKitAPI/Tests/mac/FontManagerTests.mm:
     13        (TestWebKitAPI::TEST):
     14
    1152019-07-15  Jiewen Tan  <jiewen_tan@apple.com>
    216
  • trunk/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm

    r244494 r247439  
    507507}
    508508
     509TEST(FontManagerTests, SetSelectedSystemFontAfterTogglingBold)
     510{
     511    NSFontManager *fontManager = NSFontManager.sharedFontManager;
     512
     513    auto webView = webViewForFontManagerTesting(NSFontManager.sharedFontManager, @"<body style='font-family: system-ui;' contenteditable>Foo</body>");
     514    [webView selectWord:nil];
     515    [webView waitForNextPresentationUpdate];
     516    [webView waitForNextPresentationUpdate];
     517    auto initialSelectedFont = retainPtr([fontManager selectedFont]);
     518
     519    [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
     520    [webView waitForNextPresentationUpdate];
     521    auto selectedFontAfterBolding = retainPtr([fontManager selectedFont]);
     522
     523    [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
     524    [webView waitForNextPresentationUpdate];
     525    auto selectedFontAfterUnbolding = retainPtr([fontManager selectedFont]);
     526
     527    [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
     528    [webView waitForNextPresentationUpdate];
     529    auto selectedFontAfterBoldingAgain = retainPtr([fontManager selectedFont]);
     530
     531    EXPECT_WK_STREQ([initialSelectedFont fontName], [selectedFontAfterUnbolding fontName]);
     532    EXPECT_WK_STREQ([selectedFontAfterBolding fontName], [selectedFontAfterBoldingAgain fontName]);
     533    EXPECT_FALSE([[initialSelectedFont fontName] isEqual:[selectedFontAfterBolding fontName]]);
     534    EXPECT_EQ([initialSelectedFont pointSize], 16.);
     535    EXPECT_EQ([selectedFontAfterBolding pointSize], 16.);
     536    EXPECT_EQ([selectedFontAfterUnbolding pointSize], 16.);
     537    EXPECT_EQ([selectedFontAfterBoldingAgain pointSize], 16.);
     538}
     539
    509540} // namespace TestWebKitAPI
    510541
Note: See TracChangeset for help on using the changeset viewer.