Bug 190120

Summary: [macOS] [WK2] NSFontPanel UI (font color, text decorations, font shadow) doesn't update on selection change
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bdakin, darin, enrica, ews-watchlist, mifenton, mitz, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 234770    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Depends on #234770
none
Rebase on trunk
none
For landing none

Description Wenson Hsieh 2018-09-30 15:36:25 PDT 
To reproduce:

1. Load "data:text/html,<body contenteditable>" in a WKWebView (e.g. using MiniBrowser)
2. Typing some text, select all, and open the font panel (CMD+T)
3. Modify some of the font attributes (e.g. add a font shadow, strike-through, underline, and change the font color)
4. Change the selection to any other part of the text

Observed: font attributes are default values.
Expected: font attributes should reflect attributes at the current selection.
Comment 1 Radar WebKit Bug Importer 2018-09-30 15:36:52 PDT 
<rdar://problem/44897405>
Comment 2 Wenson Hsieh 2018-09-30 18:54:35 PDT 
WebViewImpl::updateFontPanelIfNeeded already updates NSFontManager's selected font, but this isn't enough; we also need to call -setSelectedAttributes:isMultiple:.
Comment 3 Wenson Hsieh 2018-10-05 09:42:18 PDT Comment hidden (obsolete)
Comment 4 Wenson Hsieh 2021-09-20 13:45:24 PDT Comment hidden (obsolete)
Comment 5 Wenson Hsieh 2021-12-31 15:57:12 PST Comment hidden (obsolete)
Comment 6 Wenson Hsieh 2022-01-01 13:28:17 PST 
Created attachment 448162 [details]
Rebase on trunk
Comment 7 Darin Adler 2022-01-11 13:32:44 PST 
Comment on attachment 448162 [details]
Rebase on trunk

View in context: https://bugs.webkit.org/attachment.cgi?id=448162&action=review

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3060
> +        auto attributesDictionary = attributes.createDictionary();
> +        if (NSFont *nsFont = [attributesDictionary objectForKey:NSFontAttributeName]) {

Seems a small waste to build the whole dictionary just to get the font.
Comment 8 Wenson Hsieh 2022-01-12 07:54:07 PST 
Comment on attachment 448162 [details]
Rebase on trunk

View in context: https://bugs.webkit.org/attachment.cgi?id=448162&action=review

Thanks for the review!

>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3060
>> +        if (NSFont *nsFont = [attributesDictionary objectForKey:NSFontAttributeName]) {
> 
> Seems a small waste to build the whole dictionary just to get the font.

Good point — changed this so that we only create the attributes dictionary in the case where the platform NSFont exists.
Comment 9 Wenson Hsieh 2022-01-12 09:51:07 PST 
Created attachment 448957 [details]
For landing
Comment 10 EWS 2022-01-12 15:00:15 PST 
Committed r287953 (245982@main): <https://commits.webkit.org/245982@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448957 [details].