WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
224807
Clean up handling of `-webkit-inline-flex`/`-webkit-flex` CSS display values
https://bugs.webkit.org/show_bug.cgi?id=224807
Summary
Clean up handling of `-webkit-inline-flex`/`-webkit-flex` CSS display values
Tim Nguyen (:ntim)
Reported
2021-04-20 02:05:58 PDT
Context: WebKit supports the `-webkit-inline-flex`/`-webkit-flex` values for the CSS `display` property. They are aliases for `inline-flex`/`flex` specifically. Right now, those values are handled as separate `DisplayType` (see
https://webkit-search.igalia.com/webkit/rev/240c1d5ec6ecbabd0053aad3ea5e985e53198714/Source/WebCore/rendering/style/RenderStyleConstants.h#908,910
), instead we'd like to alias them directly at parse time in the `consumeDisplay` function of CSSPropertyParser.cpp. This does change the serialization (what is returned when you do `element.style.display` or `getComputedStyle(element).display`) of `-webkit-flex`/`-webkit-inline-flex` to `flex`/`inline-flex`, but this change is ok. Some tests may need to be updated to reflect this though. Steps to fix: * Add special cases directly at the beginning of the `consumeDisplay` function: ``` // Convert -webkit-flex/-webkit-inline-flex to flex/inline-flex if (range.peek().id() == CSSValueWebkitInlineFlex) return consumeIdent(range); if (range.peek().id() == CSSValueWebkitFlex) return consumeIdent(range); ``` * Move -webkit-flex/-webkit-inline-flex in Source/WebCore/css/CSSValueKeywords.in after `flow`, to avoid breaking `DisplayType display = static_cast<DisplayType>(m_value.valueID - CSSValueInline);` in CSSPrimitiveValue.cpp * Remove the WebKitFlex/WebKitInlineFlex values from the `DisplayType` enum. * Also remove all their usages too:
https://webkit-search.igalia.com/webkit/search?q=DisplayType%3A%3AWebKitFlex&path=&case=false®exp=false
https://webkit-search.igalia.com/webkit/search?q=DisplayType%3A%3AWebKitInlineFlex&path=&case=false®exp=false
* Fix all potential failing tests (You'll find those by running `Tools/Scripts/run-webkit-tests --no-retry --no-show LayoutTests/` or by submitting the patch on Bugzilla and checking EWS failures). The failures will be due tests expecting `-webkit-flex`/`-webkit-inline-flex` instead of the unprefixed version when serializing. The tests should be changed to reflect that. Please feel free to reach out on the WebKit Slack for any questions (I'm @ntim).
Attachments
Patch
(8.07 KB, patch)
2021-05-24 02:22 PDT
,
Tim Nguyen (:ntim)
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-04-20 02:29:00 PDT
<
rdar://problem/76888941
>
Tim Nguyen (:ntim)
Comment 2
2021-04-20 02:35:24 PDT
Sorry, the snippet I suggested was wrong, here's one that should be correct: + // Convert -webkit-flex/-webkit-inline-flex to flex/inline-flex + CSSValueID nextValueID = range.peek().id(); + if (nextValueID == CSSValueWebkitInlineFlex || nextValueID == CSSValueWebkitFlex) { + consumeIdent(range); + return CSSValuePool::singleton().createValue( + nextValueID == CSSValueWebkitInlineFlex ? CSSValueInlineFlex : CSSValueFlex); + }
Tim Nguyen (:ntim)
Comment 3
2021-05-24 01:31:06 PDT
I'll take this.
Tim Nguyen (:ntim)
Comment 4
2021-05-24 02:22:23 PDT
Created
attachment 429513
[details]
Patch
EWS
Comment 5
2021-05-24 06:59:26 PDT
Committed
r277949
(
238076@main
): <
https://commits.webkit.org/238076@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 429513
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug