Bug 249107 - canonicalUnitTypeForCategory should handle percent category
Summary: canonicalUnitTypeForCategory should handle percent category
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antti Koivisto
URL:
Keywords: InRadar
Depends on: 249131 249132
Blocks: 189692
  Show dependency treegraph
 
Reported: 2022-12-11 13:08 PST by Antti Koivisto
Modified: 2022-12-12 04:01 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.20 KB, patch)
2022-12-11 13:17 PST, Antti Koivisto
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.24 KB, patch)
2022-12-11 13:43 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (8.61 KB, patch)
2022-12-11 22:18 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (8.79 KB, patch)
2022-12-11 23:27 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2022-12-11 13:08:02 PST
Fix some FIXMEs.
Comment 1 Radar WebKit Bug Importer 2022-12-11 13:08:23 PST
<rdar://problem/103231020>
Comment 2 Antti Koivisto 2022-12-11 13:17:51 PST
Created attachment 463989 [details]
Patch
Comment 3 Antti Koivisto 2022-12-11 13:43:24 PST
Created attachment 463990 [details]
Patch
Comment 4 Antti Koivisto 2022-12-11 22:18:25 PST
Created attachment 464000 [details]
Patch
Comment 5 Tim Nguyen (:ntim) 2022-12-11 23:24:08 PST
Comment on attachment 464000 [details]
Patch

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

> Source/WebCore/css/typedom/CSSUnitValue.cpp:161
>      auto canonicalUnit = [] (CSSUnitType unit) {
> -        // FIXME: We probably want to change the definition of canonicalUnitTypeForCategory so this lambda isn't necessary.
> -        auto category = unitCategory(unit);
> -        switch (category) {
> -        case CSSUnitCategory::Percent:
> -            return CSSUnitType::CSS_PERCENTAGE;
> -        case CSSUnitCategory::Flex:
> -            return CSSUnitType::CSS_FR;
> -        default:
> -            break;
> -        }
> -        auto result = canonicalUnitTypeForCategory(category);
> +        auto result = canonicalUnitTypeForUnitType(unit);
>          return result == CSSUnitType::CSS_UNKNOWN ? unit : result;
>      } (m_unit);

Do we really need this lambda? Can we just call `canonicalUnitTypeForUnitType` directly ?
Comment 6 Antti Koivisto 2022-12-11 23:27:31 PST
Created attachment 464002 [details]
Patch for landing
Comment 7 EWS 2022-12-12 00:31:47 PST
Committed 257716@main (f23f94eab4ca): <https://commits.webkit.org/257716@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 464002 [details].
Comment 8 WebKit Commit Bot 2022-12-12 03:00:03 PST
Re-opened since this is blocked by bug 249131
Comment 9 Philippe Normand 2022-12-12 03:19:00 PST
Manually reverted, see https://bugs.webkit.org/show_bug.cgi?id=249133
Comment 10 Philippe Normand 2022-12-12 04:01:21 PST
Revert no longer needed.