[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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@247439 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index ff94b3c..d8f1cbe 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,43 @@
+2019-07-15 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ [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.
+
+ 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):
+
2019-07-15 Jiewen Tan <jiewen_tan@apple.com>
[iOS] SOAuthorizationSession should tell AppSSO whether the UIClient is capable of showing the extension UI
diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
index bfd4334..8410992 100644
--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
@@ -34,6 +34,7 @@
#import "AppKitSPI.h"
#import "AttributedString.h"
#import "ColorSpaceData.h"
+#import "FontInfo.h"
#import "FullscreenClient.h"
#import "GenericCallback.h"
#import "InsertTextOptions.h"
@@ -2809,9 +2810,20 @@
if (!fontPanelIsVisible && !m_page->editorState().isContentRichlyEditable)
return;
- m_page->fontAtSelection([](const String& fontName, double fontSize, bool selectionHasMultipleFonts, WebKit::CallbackBase::Error error) {
- if (NSFont *font = [NSFont fontWithName:fontName size:fontSize])
- [NSFontManager.sharedFontManager setSelectedFont:font isMultiple:selectionHasMultipleFonts];
+ m_page->fontAtSelection([](const FontInfo& fontInfo, double fontSize, bool selectionHasMultipleFonts) {
+ NSDictionary *attributeDictionary = (__bridge NSDictionary *)fontInfo.fontAttributeDictionary.get();
+ if (!attributeDictionary)
+ return;
+
+ NSFontDescriptor *descriptor = [NSFontDescriptor fontDescriptorWithFontAttributes:attributeDictionary];
+ if (!descriptor)
+ return;
+
+ NSFont *font = [NSFont fontWithDescriptor:descriptor size:fontSize];
+ if (!font)
+ return;
+
+ [NSFontManager.sharedFontManager setSelectedFont:font isMultiple:selectionHasMultipleFonts];
});
}
diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
index 7a59e4e..c9ba8c3 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.h
+++ b/Source/WebKit/UIProcess/WebPageProxy.h
@@ -282,6 +282,7 @@
struct DocumentEditingContextRequest;
struct EditingRange;
struct EditorState;
+struct FontInfo;
struct FrameInfoData;
struct InsertTextOptions;
struct InteractionInformationRequest;
@@ -337,7 +338,6 @@
#if PLATFORM(MAC)
typedef GenericCallback<const AttributedString&, const EditingRange&> AttributedStringForCharacterRangeCallback;
-typedef GenericCallback<const String&, double, bool> FontAtSelectionCallback;
#endif
#if PLATFORM(IOS_FAMILY)
@@ -788,7 +788,7 @@
#if PLATFORM(MAC)
void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::TextAlternativeWithRange>& dictationAlternatives, bool registerUndoGroup);
void attributedSubstringForCharacterRangeAsync(const EditingRange&, WTF::Function<void (const AttributedString&, const EditingRange&, CallbackBase::Error)>&&);
- void fontAtSelection(WTF::Function<void (const String&, double, bool, CallbackBase::Error)>&&);
+ void fontAtSelection(Function<void(const FontInfo&, double, bool)>&&);
void startWindowDrag();
NSWindow *platformWindow();
@@ -1869,7 +1869,6 @@
void rectForCharacterRangeCallback(const WebCore::IntRect&, const EditingRange&, CallbackID);
#if PLATFORM(MAC)
void attributedStringForCharacterRangeCallback(const AttributedString&, const EditingRange&, CallbackID);
- void fontAtSelectionCallback(const String&, double, bool, CallbackID);
#endif
#if PLATFORM(IOS_FAMILY)
void gestureCallback(const WebCore::IntPoint&, uint32_t gestureType, uint32_t gestureState, uint32_t flags, CallbackID);
diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
index 4163793..ff053df5 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
@@ -180,7 +180,6 @@
#endif
#if PLATFORM(MAC)
AttributedStringForCharacterRangeCallback(struct WebKit::AttributedString string, struct WebKit::EditingRange actualRange, WebKit::CallbackID callbackID)
- FontAtSelectionCallback(String fontName, double fontSize, bool selectioHasMultipleFonts, WebKit::CallbackID callbackID)
#endif
FontAttributesCallback(struct WebCore::FontAttributes attributes, WebKit::CallbackID callbackID)
#if PLATFORM(IOS_FAMILY)
diff --git a/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm b/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
index 87c68b3..311ec34 100644
--- a/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
+++ b/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
@@ -33,6 +33,7 @@
#import "ColorSpaceData.h"
#import "DataReference.h"
#import "EditorState.h"
+#import "FontInfo.h"
#import "InsertTextOptions.h"
#import "MenuUtilities.h"
#import "NativeWebKeyboardEvent.h"
@@ -240,28 +241,13 @@
callback->performCallbackWithReturnValue(string, actualRange);
}
-void WebPageProxy::fontAtSelection(WTF::Function<void (const String&, double, bool, CallbackBase::Error)>&& callbackFunction)
+void WebPageProxy::fontAtSelection(Function<void(const FontInfo&, double, bool)>&& callback)
{
if (!hasRunningProcess()) {
- callbackFunction(String(), 0, false, CallbackBase::Error::Unknown);
+ callback({ }, 0, false);
return;
}
-
- auto callbackID = m_callbacks.put(WTFMove(callbackFunction), m_process->throttler().backgroundActivityToken());
-
- process().send(Messages::WebPage::FontAtSelection(callbackID), m_pageID);
-}
-
-void WebPageProxy::fontAtSelectionCallback(const String& fontName, double fontSize, bool selectionHasMultipleFonts, CallbackID callbackID)
-{
- auto callback = m_callbacks.take<FontAtSelectionCallback>(callbackID);
- if (!callback) {
- // FIXME: Log error or assert.
- // this can validly happen if a load invalidated the callback, though
- return;
- }
-
- callback->performCallbackWithReturnValue(fontName, fontSize, selectionHasMultipleFonts);
+ m_process->connection()->sendWithAsyncReply(Messages::WebPage::FontAtSelection(), WTFMove(callback), m_pageID);
}
String WebPageProxy::stringSelectionForPasteboard()
diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
index 97483e0..3ee3a89 100644
--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
@@ -258,6 +258,7 @@
struct BackForwardListItemState;
struct DataDetectionResult;
struct EditorState;
+struct FontInfo;
struct InsertTextOptions;
struct InteractionInformationAtPosition;
struct InteractionInformationRequest;
@@ -810,7 +811,7 @@
#if PLATFORM(MAC)
void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::DictationAlternative>& dictationAlternativeLocations, bool registerUndoGroup = false);
void attributedSubstringForCharacterRangeAsync(const EditingRange&, CallbackID);
- void fontAtSelection(CallbackID);
+ void fontAtSelection(CompletionHandler<void(const FontInfo&, double, bool)>&&);
#endif
#if PLATFORM(COCOA) && ENABLE(SERVICE_CONTROLS)
diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
index f31ee1d..06cff4e 100644
--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
@@ -445,7 +445,7 @@
#if PLATFORM(MAC)
InsertDictatedTextAsync(String text, struct WebKit::EditingRange replacementRange, Vector<WebCore::DictationAlternative> dictationAlternatives, bool registerUndoGroup)
AttributedSubstringForCharacterRangeAsync(struct WebKit::EditingRange range, WebKit::CallbackID callbackID);
- FontAtSelection(WebKit::CallbackID callbackID);
+ FontAtSelection() -> (struct WebKit::FontInfo info, double fontSize, bool hasMultipleFonts) Async
#endif
SetAlwaysShowsHorizontalScroller(bool alwaysShowsHorizontalScroller)
diff --git a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
index 6be0582..a0a9b3d 100644
--- a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
+++ b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
@@ -33,6 +33,7 @@
#import "DataReference.h"
#import "EditingRange.h"
#import "EditorState.h"
+#import "FontInfo.h"
#import "InjectedBundleHitTestResult.h"
#import "PDFKitImports.h"
#import "PDFPlugin.h"
@@ -376,22 +377,35 @@
send(Messages::WebPageProxy::AttributedStringForCharacterRangeCallback(attributedString, rangeToSend, callbackID));
}
-void WebPage::fontAtSelection(CallbackID callbackID)
+void WebPage::fontAtSelection(CompletionHandler<void(const FontInfo&, double, bool)>&& reply)
{
- String fontName;
- double fontSize = 0;
bool selectionHasMultipleFonts = false;
- Frame& frame = m_page->focusController().focusedOrMainFrame();
-
- if (!frame.selection().selection().isNone()) {
- if (auto* font = frame.editor().fontForSelection(selectionHasMultipleFonts)) {
- if (auto ctFont = font->getCTFont()) {
- fontName = adoptCF(CTFontCopyPostScriptName(ctFont)).get();
- fontSize = CTFontGetSize(ctFont);
- }
- }
+ auto& frame = m_page->focusController().focusedOrMainFrame();
+
+ if (frame.selection().selection().isNone()) {
+ reply({ }, 0, false);
+ return;
}
- send(Messages::WebPageProxy::FontAtSelectionCallback(fontName, fontSize, selectionHasMultipleFonts, callbackID));
+
+ auto* font = frame.editor().fontForSelection(selectionHasMultipleFonts);
+ if (!font) {
+ reply({ }, 0, false);
+ return;
+ }
+
+ auto ctFont = font->getCTFont();
+ if (!ctFont) {
+ reply({ }, 0, false);
+ return;
+ }
+
+ auto fontDescriptor = adoptCF(CTFontCopyFontDescriptor(ctFont));
+ if (!fontDescriptor) {
+ reply({ }, 0, false);
+ return;
+ }
+
+ reply({ adoptCF(CTFontDescriptorCopyAttributes(fontDescriptor.get())) }, CTFontGetSize(ctFont), selectionHasMultipleFonts);
}
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 9b19636..a045022 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,17 @@
+2019-07-15 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ [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.
+
+ 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):
+
2019-07-15 Jiewen Tan <jiewen_tan@apple.com>
[iOS] SOAuthorizationSession should tell AppSSO whether the UIClient is capable of showing the extension UI
diff --git a/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm b/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm
index 723d3c0..447f28e 100644
--- a/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm
+++ b/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm
@@ -506,6 +506,37 @@
EXPECT_NULL(attributes[NSBackgroundColorAttributeName]);
}
+TEST(FontManagerTests, SetSelectedSystemFontAfterTogglingBold)
+{
+ NSFontManager *fontManager = NSFontManager.sharedFontManager;
+
+ auto webView = webViewForFontManagerTesting(NSFontManager.sharedFontManager, @"<body style='font-family: system-ui;' contenteditable>Foo</body>");
+ [webView selectWord:nil];
+ [webView waitForNextPresentationUpdate];
+ [webView waitForNextPresentationUpdate];
+ auto initialSelectedFont = retainPtr([fontManager selectedFont]);
+
+ [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
+ [webView waitForNextPresentationUpdate];
+ auto selectedFontAfterBolding = retainPtr([fontManager selectedFont]);
+
+ [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
+ [webView waitForNextPresentationUpdate];
+ auto selectedFontAfterUnbolding = retainPtr([fontManager selectedFont]);
+
+ [webView _synchronouslyExecuteEditCommand:@"ToggleBold" argument:nil];
+ [webView waitForNextPresentationUpdate];
+ auto selectedFontAfterBoldingAgain = retainPtr([fontManager selectedFont]);
+
+ EXPECT_WK_STREQ([initialSelectedFont fontName], [selectedFontAfterUnbolding fontName]);
+ EXPECT_WK_STREQ([selectedFontAfterBolding fontName], [selectedFontAfterBoldingAgain fontName]);
+ EXPECT_FALSE([[initialSelectedFont fontName] isEqual:[selectedFontAfterBolding fontName]]);
+ EXPECT_EQ([initialSelectedFont pointSize], 16.);
+ EXPECT_EQ([selectedFontAfterBolding pointSize], 16.);
+ EXPECT_EQ([selectedFontAfterUnbolding pointSize], 16.);
+ EXPECT_EQ([selectedFontAfterBoldingAgain pointSize], 16.);
+}
+
} // namespace TestWebKitAPI
#endif // PLATFORM(MAC)
