Changeset 236770 in webkit


Ignore:
Timestamp:
Oct 2, 2018 3:53:37 PM (6 years ago)
Author:
Wenson Hsieh
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):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r236762 r236770  
     12018-10-02  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [WebKit2] Format > Bold and Format > Italic don't toggle between bold and italic style
     4        https://bugs.webkit.org/show_bug.cgi?id=179787
     5        <rdar://problem/35593389>
     6
     7        Reviewed by Tim Horton.
     8
     9        Fixes the bug by relaxing our policy in WebViewImpl::updateFontManagerIfNeeded. Instead of updating only when
     10        the font panel is visible, update when either the font panel is visible, or the selection is in a richly
     11        editable area. In the latter case, an up-to-date NSFontManager state is needed in order for certain font
     12        formatting menu items, such as bold and italic, to correctly toggle state.
     13
     14        Test: FontManagerTests.ToggleBoldAndItalicWithMenuItems
     15
     16        * UIProcess/Cocoa/WebViewImpl.h:
     17        * UIProcess/Cocoa/WebViewImpl.mm:
     18        (-[WKWindowVisibilityObserver observeValueForKeyPath:ofObject:change:context:]):
     19        (WebKit::WebViewImpl::selectionDidChange):
     20        (WebKit::WebViewImpl::updateFontManagerIfNeeded):
     21
     22        Rename updateFontPanelIfNeeded to updateFontManagerIfNeeded, to reflect the new behavior.
     23
     24        (WebKit::WebViewImpl::changeFontAttributesFromSender):
     25        (WebKit::WebViewImpl::changeFontFromFontManager):
     26        (WebKit::WebViewImpl::updateFontPanelIfNeeded): Deleted.
     27
    1282018-10-02  Alex Christensen  <achristensen@webkit.org>
    229

  • trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h

    r236724 r236770  
    324324   
    325325    void didBecomeEditable();
    326     void updateFontPanelIfNeeded();
     326    void updateFontManagerIfNeeded();
    327327    void changeFontFromFontManager();
    328328    void changeFontAttributesFromSender(id);

  • trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

    r236724 r236770  
    375375
    376376    if ([keyPath isEqualToString:@"visible"] && [NSFontPanel sharedFontPanelExists] && object == [NSFontPanel sharedFontPanel]) {
    377         _impl->updateFontPanelIfNeeded();
     377        _impl->updateFontManagerIfNeeded();
    378378        return;
    379379    }
    … …  
    27372737void WebViewImpl::selectionDidChange()
    27382738{
    2739     updateFontPanelIfNeeded();
     2739    updateFontManagerIfNeeded();
    27402740    if (!m_isHandlingAcceptedCandidate)
    27412741        m_softSpaceRange = NSMakeRange(NSNotFound, 0);
    … …  
    27882788}
    27892789
    2790 void WebViewImpl::updateFontPanelIfNeeded()
    2791 {
    2792     const EditorState& editorState = m_page->editorState();
    2793     if (editorState.selectionIsNone || !editorState.isContentEditable)
    2794         return;
    2795     if ([NSFontPanel sharedFontPanelExists] && [[NSFontPanel sharedFontPanel] isVisible]) {
    2796         m_page->fontAtSelection([](const String& fontName, double fontSize, bool selectionHasMultipleFonts, WebKit::CallbackBase::Error error) {
    2797             NSFont *font = [NSFont fontWithName:fontName size:fontSize];
    2798             if (font)
    2799                 [[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:selectionHasMultipleFonts];
    2800         });
    2801     }
     2790void WebViewImpl::updateFontManagerIfNeeded()
     2791{
     2792    BOOL fontPanelIsVisible = NSFontPanel.sharedFontPanelExists && NSFontPanel.sharedFontPanel.visible;
     2793    if (!fontPanelIsVisible && !m_page->editorState().isContentRichlyEditable)
     2794        return;
     2795
     2796    m_page->fontAtSelection([](const String& fontName, double fontSize, bool selectionHasMultipleFonts, WebKit::CallbackBase::Error error) {
     2797        if (NSFont *font = [NSFont fontWithName:fontName size:fontSize])
     2798            [NSFontManager.sharedFontManager setSelectedFont:font isMultiple:selectionHasMultipleFonts];
     2799    });
    28022800}
    28032801
    … …  
    28462844
    28472845    m_page->changeFontAttributes(WebCore::computedFontAttributeChanges(NSFontManager.sharedFontManager, sender));
    2848     updateFontPanelIfNeeded();
     2846    updateFontManagerIfNeeded();
    28492847}
    28502848
    … …  
    28562854
    28572855    m_page->changeFont(WebCore::computedFontChanges(NSFontManager.sharedFontManager));
    2858     updateFontPanelIfNeeded();
     2856    updateFontManagerIfNeeded();
    28592857}
    28602858

  • trunk/Tools/ChangeLog

    r236764 r236770  
     12018-10-02  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [WebKit2] Format > Bold and Format > Italic don't toggle between bold and italic style
     4        https://bugs.webkit.org/show_bug.cgi?id=179787
     5        <rdar://problem/35593389>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a test to verify that NSFontManager's selected font is updated when applying italic and bold styles using
     10        menu items.
     11
     12        * TestWebKitAPI/Tests/mac/FontManagerTests.mm:
     13        (webViewForFontManagerTesting):
     14        (menuItemCellForFontAction):
     15        (TestWebKitAPI::TEST):
     16
    1172018-10-02  Chris Dumez  <cdumez@apple.com>
    218

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

    r236724 r236770  
    121121        "</body><script>document.body.addEventListener('input', event => lastInputEvent = event)</script>"];
    122122    [webView stringByEvaluatingJavaScript:@"document.body.focus()"];
     123    [webView _setEditable:YES];
    123124    fontManager.target = webView.get();
    124125    return webView;
    125126}
    126127
    127 static RetainPtr<NSMenuItemCell> menuItemCellForFontAction(NSFontAction action)
     128static RetainPtr<NSMenuItemCell> menuItemCellForFontAction(NSUInteger tag)
    128129{
    129130    auto menuItem = adoptNS([[NSMenuItem alloc] init]);
    130131    auto menuItemCell = adoptNS([[NSMenuItemCell alloc] init]);
    131132    [menuItemCell setMenuItem:menuItem.get()];
    132     [menuItemCell setTag:action];
     133    [menuItemCell setTag:tag];
    133134    return menuItemCell;
    134135}
    135136
    136137namespace TestWebKitAPI {
     138
     139TEST(FontManagerTests, ToggleBoldAndItalicWithMenuItems)
     140{
     141    NSFontManager *fontManager = NSFontManager.sharedFontManager;
     142    auto webView = webViewForFontManagerTesting(fontManager);
     143
     144    [webView selectWord:nil];
     145    [fontManager addFontTrait:menuItemCellForFontAction(NSBoldFontMask).autorelease()];
     146    EXPECT_WK_STREQ("bold", [webView stylePropertyAtSelectionStart:@"font-weight"]);
     147    EXPECT_WK_STREQ("bold", [webView stylePropertyAtSelectionEnd:@"font-weight"]);
     148    EXPECT_WK_STREQ("Times-Bold", [fontManager selectedFont].fontName);
     149
     150    [fontManager addFontTrait:menuItemCellForFontAction(NSUnboldFontMask).autorelease()];
     151    EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionStart:@"font-weight"]);
     152    EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionEnd:@"font-weight"]);
     153    EXPECT_WK_STREQ("Times-Roman", [fontManager selectedFont].fontName);
     154
     155    [fontManager addFontTrait:menuItemCellForFontAction(NSItalicFontMask).autorelease()];
     156    EXPECT_WK_STREQ("italic", [webView stylePropertyAtSelectionStart:@"font-style"]);
     157    EXPECT_WK_STREQ("italic", [webView stylePropertyAtSelectionEnd:@"font-style"]);
     158    EXPECT_WK_STREQ("Times-Italic", [fontManager selectedFont].fontName);
     159
     160    [fontManager addFontTrait:menuItemCellForFontAction(NSUnitalicFontMask).autorelease()];
     161    EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionStart:@"font-style"]);
     162    EXPECT_WK_STREQ("normal", [webView stylePropertyAtSelectionEnd:@"font-style"]);
     163    EXPECT_WK_STREQ("Times-Roman", [fontManager selectedFont].fontName);
     164}
    137165
    138166TEST(FontManagerTests, ChangeFontSizeWithMenuItems)
Note: See TracChangeset for help on using the changeset viewer.