Bug 190120 - [macOS] [WK2] NSFontPanel UI (font color, text decorations, font shadow) doesn't update on selection change
Summary: [macOS] [WK2] NSFontPanel UI (font color, text decorations, font shadow) does...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on: 234770
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-30 15:36 PDT by Wenson Hsieh
Modified: 2022-01-12 15:00 PST (History)
10 users (show)

See Also:


Attachments
Patch (44.46 KB, patch)
2018-10-05 09:42 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Depends on #234770 (41.75 KB, patch)
2021-12-31 15:57 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Rebase on trunk (41.76 KB, patch)
2022-01-01 13:28 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
For landing (41.54 KB, patch)
2022-01-12 09:51 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].