Steps to reproduce: 1. Open MiniBrowser 2. Choose File > New WebKit2 Editor 3. Click inside the new window and type some text 4. Select some of the text 5. Choose Format > Font > Bold or Italic or Bigger Results: The font doesn’t change Further steps: 6. Choose Format > Font > Show Fonts 7. Repeat step 5 Results: The font changes Further steps: 8. Choose some other font (different size and typeface) in the Fonts panel and close it 9. Open a new WebKit2 Editor window and repeat steps 3-5 Results: The font changes but takes on properties of the font last selected in the now-closed Fonts panel Notes: -[WKWebView changeFont:] seems to be all about the Font panel, whereas -[WebView changeFont:] seems to rely more on the font manager.
<rdar://problem/35593389>
r235748 addresses most of this. Before r235748, all of the font-modifying menu items were no-ops; after r235748, B/I/U menu items work the first time, but fail to toggle B/I/U state. I'll use this to track a followup.
(In reply to Wenson Hsieh from comment #2) > r235748 addresses most of this. Before r235748, all of the font-modifying > menu items were no-ops; after r235748, B/I/U menu items work the first time, > but fail to toggle B/I/U state. Correction: the problem is relevant to bold and italic, but underline works.
Created attachment 351404 [details] Patch
Comment on attachment 351404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351404&action=review > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2796 > + if (m_page->isEditable() || (NSFontPanel.sharedFontPanelExists && NSFontPanel.sharedFontPanel.visible)) { Can you explain the m_page->isEditable() test here? It seems like it might help with the steps in comment 0, but we’d still have the bug in a MiniBrowser window navigated to data:text/html,<div contenteditable>.
(In reply to mitz from comment #5) > Comment on attachment 351404 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351404&action=review > > > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2796 > > + if (m_page->isEditable() || (NSFontPanel.sharedFontPanelExists && NSFontPanel.sharedFontPanel.visible)) { > > Can you explain the m_page->isEditable() test here? It seems like it might > help with the steps in comment 0, but we’d still have the bug in a > MiniBrowser window navigated to data:text/html,<div contenteditable>. Yeah, this patch isn't intended fix the bug for a stock WKWebView; only one that's been made editable via SPI, à la Mail. I'm using the check for editability here as a cue that we should be aggressive in keeping NSFontManager up to date. An alternate solution I'd considered was just checking if we're in a richly editable area instead, which would fix this for a normal WKWebView as well. Would you suggest this second approach?
(In reply to Wenson Hsieh from comment #6) > (In reply to mitz from comment #5) > > Comment on attachment 351404 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=351404&action=review > > > > > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2796 > > > + if (m_page->isEditable() || (NSFontPanel.sharedFontPanelExists && NSFontPanel.sharedFontPanel.visible)) { > > > > Can you explain the m_page->isEditable() test here? It seems like it might > > help with the steps in comment 0, but we’d still have the bug in a > > MiniBrowser window navigated to data:text/html,<div contenteditable>. > > Yeah, this patch isn't intended fix the bug for a stock WKWebView; only one > that's been made editable via SPI, à la Mail. > > I'm using the check for editability here as a cue that we should be > aggressive in keeping NSFontManager up to date. An alternate solution I'd > considered was just checking if we're in a richly editable area instead, > which would fix this for a normal WKWebView as well. > > Would you suggest this second approach? Assuming it makes WKWebView behave as well as WebView does, it seems reasonable.
Created attachment 351423 [details] Patch
Comment on attachment 351423 [details] Patch Clearing flags on attachment: 351423 Committed r236770: <https://trac.webkit.org/changeset/236770>
All reviewed patches have been landed. Closing bug.