Bug 258975 – Web Inspector: Font Panel: CSS font property values marked !important don't get overridden when using the interactive editing controls.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Jeff Fortin]

Created attachment 466977 [details]
Screenshot of the existing font panel

Testing with Epiphany 44.3 and WebKitGTK 2.40.3 on Fedora 38, I see the web inspector has a "Font" panel, which is pretty nice, except that it does not allow you to change any of the values there. The only widget in that screenshot that seems interactive (kind of) is the weight slider, but it actually ignores what you when you drag it, and it resets into place.

It would be pretty neat if one could modify any font properties for the selected item in the web page's DOM using the inspector; Chrome/Chromium doesn't have that feature (it doesn't even have a font inspector that I could find), but Firefox does.

I have asked someone to test with Safari on macOS and reportedly the font panel in its inspector has the same limitation as WebKitGTK on Linux.

[Razvan Caliman]

Can you please share a link to a web page where this issue occurs?

The interactive controls in the Font sidebar show up for variation axes of variable fonts applied to the selected node (the "wght" variation axis maps to the `font-weight` CSS property).

They do work correctly in the latest Safari Technology Preview 173 when tested on the sample text at https://v-fonts.com/fonts/sf-pro 

Dragging the slider writes the appropriate CSS property/value pair (the specific CSS `font-*` property or `font-variation-settings`) to the node's inline style.

[Michael Catanzaro]

Works for me too.

Looks like these controls are for managing font variation properties. I think this only works for particular fonts.

[Jeff Fortin]

My test case in the screenshot above is https://themes.tiki.org ; on that page, right-clicking on the main heading ("Themes.tiki.org") or the text underneath it, which shows up with the "Cantarell" font in GNOME, nothing there can be edited, not even the 4rem size...

As for https://v-fonts.com/fonts/sf-pro :

* If I right-click the main big test text from that page and "Inspect element" to see its Font pane in the inspector, I can change the Weight, Width, opsz and GRAD, but not any of the other properties like size, style, capitals, etc.

* If I right-click the description text on that page ("SF Pro is the core of...") and inspect it the same way, I can change even less things, only the weight (wght) and the optical size (opsz)...

* Firefox, in comparison, is able to change the size, line height, spacing, italic or not, weight, optical size, "instance" (which seems to control variants/thickness/etc.)

[Razvan Caliman]

(In reply to Jeff Fortin from comment #3)
> My test case in the screenshot above is https://themes.tiki.org ; on that
> page, right-clicking on the main heading ("Themes.tiki.org") or the text
> underneath it, which shows up with the "Cantarell" font in GNOME, nothing
> there can be edited, not even the 4rem size...

Thanks for providing an URL to test!

I can confirm that, indeed, using the slider for Weight in the Font sidebar panel won't change the value of the `font-weight` CSS property on the inline style for the element:

```
<div class="display-3 fw-medium my-5" style="word-break: break-word;">Themes.tiki.org</div>
```

A summary investigation shows there's a conflict with `font-weight 500 !important` set by the `fw-medium` CSS class. This is indeed a problem. I'll retitle this bug to identify this.

> As for https://v-fonts.com/fonts/sf-pro :
> 
> * If I right-click the main big test text from that page and "Inspect
> element" to see its Font pane in the inspector, I can change the Weight,
> Width, opsz and GRAD, but not any of the other properties like size, style,
> capitals, etc.


That is expected behavior. There is currently no implementation for specialized editing controls for font size, style or any of the font feature properties.


> * Firefox, in comparison, is able to change the size, line height, spacing,
> italic or not, weight, optical size, "instance" (which seems to control
> variants/thickness/etc.)

This sounds like a feature request. Indeed, Web Inspector does not yet support interactive editing for all font properties in the Font sidebar; neither does Firefox DevTools ;)

Please file separate issues for the properties you'd like to be editable. It helps a lot if you can describe your use case for needing specialized editing controls for each property.

(There is already an entry for named instances of variable fonts in Bug 249536)

Thanks for filing this issue!

[Radar WebKit Bug Importer]

<rdar://problem/112080113>

[Razvan Caliman]

Created attachment 467023 [details]
Test case

[Razvan Caliman]

(In reply to Razvan Caliman from comment #4)
> I can confirm that, indeed, using the slider for Weight in the Font sidebar
> panel won't change the value of the `font-weight` CSS property on the inline
> style for the element:
> 
> ```
> <div class="display-3 fw-medium my-5" style="word-break:
> break-word;">Themes.tiki.org</div>
> ```
> 
> A summary investigation shows there's a conflict with `font-weight 500
> !important` set by the `fw-medium` CSS class. This is indeed a problem. I'll
> retitle this bug to identify this.

To fix this issue, we need to ensure that:

1) the correct `WI.CSSProperty` is returned by `WI.FontStyles._effectiveWritablePropertyForName()` when there are multiple properties with the same name and/or the CSS property value contains `!important`

2) tf the CSS property has `!important`, the new value written by `WI.FontStyles.writeFontVariation()` preserves that so the property continues to apply.

[Frances Cornwall]

I replicated this bug and am currently working on a fix for this.

[Frances Cornwall]

I added

if ((targetPropertyName === "font-weight" || targetPropertyName === "font-stretch") && !targetPropertyValue.includes("!important")) {
            targetPropertyValue += "!important";
        }

in writeFontVariation() in FontStyles.js to start addressing the bug. However, !important shows in the web inspector. I confirmed with the test case on https://themes.tiki.org.

[Razvan Caliman]

(In reply to Frances from comment #9)
> I added
> 
> if ((targetPropertyName === "font-weight" || targetPropertyName ===
> "font-stretch") && !targetPropertyValue.includes("!important")) {
>             targetPropertyValue += "!important";
>         }
> 

There's no need to restrict this behavior by property name. Any CSS property, currently written to or writable in the future, can have `!important` in its value.

`targetPropertyValue` is what Web Inspector is trying to write back to the CSS property.

Notice that `WI.CSSProperty` model has a an `important` getter will already tell you whether the CSS property value contains the `!important` modifier.

`WI.FontStyles.axisValueToFontPropertyValue()` is just a translation mechanism; it has no knowledge about the original CSS property.

The thing you want to operate on is the `WI.CSSProperty` model, `cssProperty`, from:
https://github.com/WebKit/WebKit/blob/a59c221d09e9f89a0f3ab013e96fc9ebe33cbd80/Source/WebInspectorUI/UserInterface/Models/FontStyles.js#L140

```
let cssProperty = this._effectiveWritablePropertyForName(targetPropertyName, createIfMissing);
```

You should also ensure that `this._effectiveWritablePropertyForName()` returns the correct model if there are multiple candidates with the same property name but one or more have `!important` in their values.

[Frances Cornwall]

Pull request: https://github.com/WebKit/WebKit/pull/20015

[Frances Cornwall]

I updated the pr and it handles the following edge cases including existing CSS properties on the inline style that overwrite each other with either multiple font-weights, !important and more font-weights, or disabled font-weight styles.

[Frances Cornwall]

Pull request: https://github.com/WebKit/WebKit/pull/21557

[EWS]

Committed 275389@main (793932ceb15a): <https://commits.webkit.org/275389@main>

Reviewed commits have been landed. Closing PR #21557 and removing active labels.