- Revision
- 236770
- Author
- wenson_hs...@apple.com
- Date
- 2018-10-02 15:53:37 -0700 (Tue, 02 Oct 2018)
Log Message
[WebKit2] Format > Bold and Format > Italic don't toggle between bold and italic style
https://bugs.webkit.org/show_bug.cgi?id=179787
<rdar://problem/35593389>
Reviewed by Tim Horton.
Source/WebKit:
Fixes the bug by relaxing our policy in WebViewImpl::updateFontManagerIfNeeded. Instead of updating only when
the font panel is visible, update when either the font panel is visible, or the selection is in a richly
editable area. In the latter case, an up-to-date NSFontManager state is needed in order for certain font
formatting menu items, such as bold and italic, to correctly toggle state.
Test: FontManagerTests.ToggleBoldAndItalicWithMenuItems
* UIProcess/Cocoa/WebViewImpl.h:
* UIProcess/Cocoa/WebViewImpl.mm:
(-[WKWindowVisibilityObserver observeValueForKeyPath:ofObject:change:context:]):
(WebKit::WebViewImpl::selectionDidChange):
(WebKit::WebViewImpl::updateFontManagerIfNeeded):
Rename updateFontPanelIfNeeded to updateFontManagerIfNeeded, to reflect the new behavior.
(WebKit::WebViewImpl::changeFontAttributesFromSender):
(WebKit::WebViewImpl::changeFontFromFontManager):
(WebKit::WebViewImpl::updateFontPanelIfNeeded): Deleted.
Tools:
Add a test to verify that NSFontManager's selected font is updated when applying italic and bold styles using
menu items.
* TestWebKitAPI/Tests/mac/FontManagerTests.mm:
(webViewForFontManagerTesting):
(menuItemCellForFontAction):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (236769 => 236770)
--- trunk/Source/WebKit/ChangeLog 2018-10-02 22:53:33 UTC (rev 236769)
+++ trunk/Source/WebKit/ChangeLog 2018-10-02 22:53:37 UTC (rev 236770)
@@ -1,3 +1,30 @@
+2018-10-02 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [WebKit2] Format > Bold and Format > Italic don't toggle between bold and italic style
+ https://bugs.webkit.org/show_bug.cgi?id=179787
+ <rdar://problem/35593389>
+
+ Reviewed by Tim Horton.
+
+ Fixes the bug by relaxing our policy in WebViewImpl::updateFontManagerIfNeeded. Instead of updating only when
+ the font panel is visible, update when either the font panel is visible, or the selection is in a richly
+ editable area. In the latter case, an up-to-date NSFontManager state is needed in order for certain font
+ formatting menu items, such as bold and italic, to correctly toggle state.
+
+ Test: FontManagerTests.ToggleBoldAndItalicWithMenuItems
+
+ * UIProcess/Cocoa/WebViewImpl.h:
+ * UIProcess/Cocoa/WebViewImpl.mm:
+ (-[WKWindowVisibilityObserver observeValueForKeyPath:ofObject:change:context:]):
+ (WebKit::WebViewImpl::selectionDidChange):
+ (WebKit::WebViewImpl::updateFontManagerIfNeeded):
+
+ Rename updateFontPanelIfNeeded to updateFontManagerIfNeeded, to reflect the new behavior.
+
+ (WebKit::WebViewImpl::changeFontAttributesFromSender):
+ (WebKit::WebViewImpl::changeFontFromFontManager):
+ (WebKit::WebViewImpl::updateFontPanelIfNeeded): Deleted.
+
2018-10-02 Alex Christensen <achristen...@webkit.org>
Remove ParsedURLString
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h (236769 => 236770)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h 2018-10-02 22:53:33 UTC (rev 236769)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h 2018-10-02 22:53:37 UTC (rev 236770)
@@ -323,7 +323,7 @@
void selectionDidChange();
void didBecomeEditable();
- void updateFontPanelIfNeeded();
+ void updateFontManagerIfNeeded();
void changeFontFromFontManager();
void changeFontAttributesFromSender(id);
void changeFontColorFromSender(id);
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (236769 => 236770)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm 2018-10-02 22:53:33 UTC (rev 236769)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm 2018-10-02 22:53:37 UTC (rev 236770)
@@ -374,7 +374,7 @@
}
if ([keyPath isEqualToString:@"visible"] && [NSFontPanel sharedFontPanelExists] && object == [NSFontPanel sharedFontPanel]) {
- _impl->updateFontPanelIfNeeded();
+ _impl->updateFontManagerIfNeeded();
return;
}
if ([keyPath isEqualToString:@"contentLayoutRect"] || [keyPath isEqualToString:@"titlebarAppearsTransparent"])
@@ -2736,7 +2736,7 @@
void WebViewImpl::selectionDidChange()
{
- updateFontPanelIfNeeded();
+ updateFontManagerIfNeeded();
if (!m_isHandlingAcceptedCandidate)
m_softSpaceRange = NSMakeRange(NSNotFound, 0);
#if HAVE(TOUCH_BAR)
@@ -2787,18 +2787,16 @@
});
}
-void WebViewImpl::updateFontPanelIfNeeded()
+void WebViewImpl::updateFontManagerIfNeeded()
{
- const EditorState& editorState = m_page->editorState();
- if (editorState.selectionIsNone || !editorState.isContentEditable)
+ BOOL fontPanelIsVisible = NSFontPanel.sharedFontPanelExists && NSFontPanel.sharedFontPanel.visible;
+ if (!fontPanelIsVisible && !m_page->editorState().isContentRichlyEditable)
return;
- if ([NSFontPanel sharedFontPanelExists] && [[NSFontPanel sharedFontPanel] isVisible]) {
- m_page->fontAtSelection([](const String& fontName, double fontSize, bool selectionHasMultipleFonts, WebKit::CallbackBase::Error error) {
- NSFont *font = [NSFont fontWithName:fontName size:fontSize];
- if (font)
- [[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:selectionHasMultipleFonts];
- });
- }
+
+ 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];
+ });
}
void WebViewImpl::typingAttributesWithCompletionHandler(void(^completion)(NSDictionary<NSString *, id> *))
@@ -2845,7 +2843,7 @@
return;
m_page->changeFontAttributes(WebCore::computedFontAttributeChanges(NSFontManager.sharedFontManager, sender));
- updateFontPanelIfNeeded();
+ updateFontManagerIfNeeded();
}
void WebViewImpl::changeFontFromFontManager()
@@ -2855,7 +2853,7 @@
return;
m_page->changeFont(WebCore::computedFontChanges(NSFontManager.sharedFontManager));
- updateFontPanelIfNeeded();
+ updateFontManagerIfNeeded();
}
static NSMenuItem *menuItem(id <NSValidatedUserInterfaceItem> item)
Modified: trunk/Tools/ChangeLog (236769 => 236770)
--- trunk/Tools/ChangeLog 2018-10-02 22:53:33 UTC (rev 236769)
+++ trunk/Tools/ChangeLog 2018-10-02 22:53:37 UTC (rev 236770)
@@ -1,3 +1,19 @@
+2018-10-02 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [WebKit2] Format > Bold and Format > Italic don't toggle between bold and italic style
+ https://bugs.webkit.org/show_bug.cgi?id=179787
+ <rdar://problem/35593389>
+
+ Reviewed by Tim Horton.
+
+ Add a test to verify that NSFontManager's selected font is updated when applying italic and bold styles using
+ menu items.
+
+ * TestWebKitAPI/Tests/mac/FontManagerTests.mm:
+ (webViewForFontManagerTesting):
+ (menuItemCellForFontAction):
+ (TestWebKitAPI::TEST):
+
2018-10-02 Chris Dumez <cdu...@apple.com>
REGRESSION: TestWebKitAPI.WKWebView.ProvisionalURLNotChange API test is failing
Modified: trunk/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm (236769 => 236770)
--- trunk/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm 2018-10-02 22:53:33 UTC (rev 236769)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/FontManagerTests.mm 2018-10-02 22:53:37 UTC (rev 236770)
@@ -120,21 +120,49 @@
"<span id='foo'>foo</span> <span id='bar'>bar</span> <span id='baz'>baz</span>"
"</body><script>document.body.addEventListener('input', event => lastInputEvent = event)</script>"];
[webView stringByEvaluatingJavaScript:@"document.body.focus()"];
+ [webView _setEditable:YES];
fontManager.target = webView.get();
return webView;
}
-static RetainPtr<NSMenuItemCell> menuItemCellForFontAction(NSFontAction action)
+static RetainPtr<NSMenuItemCell> menuItemCellForFontAction(NSUInteger tag)
{
auto menuItem = adoptNS([[NSMenuItem alloc] init]);
auto menuItemCell = adoptNS([[NSMenuItemCell alloc] init]);
[menuItemCell setMenuItem:menuItem.get()];
- [menuItemCell setTag:action];
+ [menuItemCell setTag:tag];
return menuItemCell;
}
namespace TestWebKitAPI {
+TEST(FontManagerTests, ToggleBoldAndItalicWithMenuItems)
+{
+ NSFontManager *fontManager = NSFontManager.sharedFontManager;
+ auto webView = webViewForFontManagerTesting(fontManager);
+
+ [webView selectWord:nil];
+ [fontManager addFontTrait:menuItemCellForFontAction(NSBoldFontMask).autorelease()];
+ EXPECT_WK_STREQ("bold", [webView stylePropertyAtSelectionStart:@"font-weight"]);
+ EXPECT_WK_STREQ("bold", [webView stylePropertyAtSelectionEnd:@"font-weight"]);
+ EXPECT_WK_STREQ("Times-Bold", [fontManager selectedFont].fontName);
+
+ [fontManager addFontTrait:menuItemCellForFontAction(NSUnboldFontMask).autorelease()];
+ EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionStart:@"font-weight"]);
+ EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionEnd:@"font-weight"]);
+ EXPECT_WK_STREQ("Times-Roman", [fontManager selectedFont].fontName);
+
+ [fontManager addFontTrait:menuItemCellForFontAction(NSItalicFontMask).autorelease()];
+ EXPECT_WK_STREQ("italic", [webView stylePropertyAtSelectionStart:@"font-style"]);
+ EXPECT_WK_STREQ("italic", [webView stylePropertyAtSelectionEnd:@"font-style"]);
+ EXPECT_WK_STREQ("Times-Italic", [fontManager selectedFont].fontName);
+
+ [fontManager addFontTrait:menuItemCellForFontAction(NSUnitalicFontMask).autorelease()];
+ EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionStart:@"font-style"]);
+ EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionEnd:@"font-style"]);
+ EXPECT_WK_STREQ("Times-Roman", [fontManager selectedFont].fontName);
+}
+
TEST(FontManagerTests, ChangeFontSizeWithMenuItems)
{
NSFontManager *fontManager = NSFontManager.sharedFontManager;
