Bug 231566 - Update CSSDocumentation.js with latest upstream data to fix wrong documentation for block-size
Summary: Update CSSDocumentation.js with latest upstream data to fix wrong documentati...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Safari 15
Hardware: Mac (Intel) macOS 11
: P2 Normal
Assignee: Razvan Caliman
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-12 01:50 PDT by Paul Li
Modified: 2021-11-08 08:58 PST (History)
7 users (show)

See Also:


Attachments
inspect element & check its documentation (377.71 KB, image/png)
2021-10-12 01:50 PDT, Paul Li
no flags Details
[fast-cq] Patch 1.0 (63.07 KB, patch)
2021-10-28 05:59 PDT, Razvan Caliman
no flags Details | Formatted Diff | Diff
Patch 1.1 (20.13 KB, patch)
2021-11-03 14:51 PDT, Razvan Caliman
no flags Details | Formatted Diff | Diff
[fast-cq] Patch 1.2 (19.93 KB, patch)
2021-11-05 09:23 PDT, Razvan Caliman
no flags Details | Formatted Diff | Diff
[fast-cq] Patch 1.3 (20.04 KB, patch)
2021-11-08 08:47 PST, Razvan Caliman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Li 2021-10-12 01:50:26 PDT
Created attachment 440910 [details]
inspect element & check its documentation

Steps to reproduce the problem:
1. Turn on devTool & inspect element to check its CSS property.
2. Focus on "block-size" property and click the hint to see its documentation.
3. The hint indicates block-size is Logical 'width'.

What is the expected behavior?
The hint should indicates block-size is Logical 'height'.

What went wrong?
The hint should indicates block-size is Logical 'width'.
Comment 1 Radar WebKit Bug Importer 2021-10-19 01:51:16 PDT
<rdar://problem/84406443>
Comment 2 Razvan Caliman 2021-10-26 07:43:48 PDT
This has been fixed upstream in https://github.com/microsoft/vscode-custom-data/issues/27

We need to pull the latest and update: 
Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js
Comment 3 Razvan Caliman 2021-10-28 05:59:56 PDT
Created attachment 442698 [details]
[fast-cq] Patch 1.0

Run script added in Bug 232433 to update CSSDocumentation.js with latest upstream data which contains fixes for the descriptions of logical properties for sizing
Comment 4 Patrick Angle 2021-10-28 09:04:47 PDT
Comment on attachment 442698 [details]
[fast-cq] Patch 1.0

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

> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:140
> +        "description": "Processes an elementâs rendering before it is displayed in the document, by applying one or more filter effects."

There seem to be encoding issues throughout where the data shouldn't have changed, but ends up re/mis-encoded with the new script. I've also left a comment on the generator script patch.

> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:2616
> -};
> +}

Another generator issue/NIT is that we should keep the trailing semicolon.
Comment 5 Devin Rousso 2021-11-01 11:20:51 PDT
Comment on attachment 442698 [details]
[fast-cq] Patch 1.0

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

>> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:140
>> +        "description": "Processes an elementâs rendering before it is displayed in the document, by applying one or more filter effects."
> 
> There seem to be encoding issues throughout where the data shouldn't have changed, but ends up re/mis-encoded with the new script. I've also left a comment on the generator script patch.

yeah in general please keep all JS/CSS files to be UTF-8 encoded as that's more efficient for parsing (and it's nicer in bugzilla review tool as well)

> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:304
> +    "accent-color": {

It's odd for a bug that specifically calls out `block-size` to have changes for other unrelated properties.  Perhaps this bug should be renamed to something more generic and have `block-size` just be an example of the motivation for the change?
Comment 6 Razvan Caliman 2021-11-03 14:47:59 PDT
Per comment #5:

Retitled as:
"Update CSSDocumentation.js with latest upstream data to fix wrong documentation for block-size"

Was:
"Inline CSS documentation wrong for block-size"
Comment 7 Razvan Caliman 2021-11-03 14:51:19 PDT
Created attachment 443243 [details]
Patch 1.1

Run script added in Bug 232433 to update CSSDocumentation.js. Addressed code review feedback
Comment 8 Patrick Angle 2021-11-05 09:10:39 PDT
Comment on attachment 443243 [details]
Patch 1.1

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

> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:2617
> -};
> +};

Style: Missing newline at end of file (probably need fixed in the script). Otherwise LGTM.
Comment 9 Razvan Caliman 2021-11-05 09:23:34 PDT
Created attachment 443403 [details]
[fast-cq] Patch 1.2

Add newline at the end
Comment 10 Devin Rousso 2021-11-05 10:31:11 PDT
Comment on attachment 443403 [details]
[fast-cq] Patch 1.2

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

r=me

> Source/WebInspectorUI/ChangeLog:3
> +        Inline CSS documentation wrong for block-size

this should be updated to match the new bug title

> Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:1
> +// This file is generated by update-css-documentation. Do not edit.

NIT: Id suggest putting the full path from the root of the WebKit source tree so that it's explicitly clear what you're referring to
```
    // DO NOT EDIT THIS FILE. It is automatically generated by the script: Source/WebInspectorUI/Scripts/update-css-documentation.py
```
Comment 11 Razvan Caliman 2021-11-08 08:47:49 PST
Created attachment 443552 [details]
[fast-cq] Patch 1.3

Carry over R+. Regenerate file with the latest version of the script from Bug 232433. Fix Changelog
Comment 12 EWS 2021-11-08 08:58:22 PST
Committed r285405 (243962@main): <https://commits.webkit.org/243962@main>

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