FitText: Update the last component of `text-fit` property [chromium/src : main]

0 views
Skip to first unread message

Kent Tamura (Gerrit)

unread,
Jan 19, 2026, 10:05:45 PM (4 days ago) Jan 19
to Kent Tamura, Koji Ishii, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Kent Tamura and Koji Ishii

Kent Tamura voted and added 1 comment

Votes added by Kent Tamura

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Kent Tamura . resolved

kojii@, would you review this please?

Open in Gerrit

Related details

Attention is currently required from:
  • Kent Tamura
  • Koji Ishii
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I4e9e539d6564f09b7c696c43b077f4c77ff4fde2
Gerrit-Change-Number: 7495731
Gerrit-PatchSet: 2
Gerrit-Owner: Kent Tamura <tk...@chromium.org>
Gerrit-Reviewer: Kent Tamura <tk...@chromium.org>
Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-Attention: Koji Ishii <ko...@chromium.org>
Gerrit-Attention: Kent Tamura <tk...@chromium.org>
Gerrit-Comment-Date: Tue, 20 Jan 2026 03:05:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Koji Ishii (Gerrit)

unread,
Jan 19, 2026, 11:49:59 PM (4 days ago) Jan 19
to Kent Tamura, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Kent Tamura

Koji Ishii voted and added 2 comments

Votes added by Koji Ishii

Code-Review+1

2 comments

Patchset-level comments
File-level comment, Patchset 3 (Latest):
Koji Ishii . resolved

lgtm w/nit

File third_party/blink/renderer/core/style/fit_text.h
Line 49, Patchset 3 (Latest): std::optional<double> ScaleFactorLimit() const { return scale_factor_limit_; }
Koji Ishii . unresolved

nit: Not strong, but I think `float` has enough precision, and can save 4 bytes on `ComputedStyle`. I checked two examples, `Color` and `DynamicRangeLimit`, both stores as floats.

Open in Gerrit

Related details

Attention is currently required from:
  • Kent Tamura
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I4e9e539d6564f09b7c696c43b077f4c77ff4fde2
    Gerrit-Change-Number: 7495731
    Gerrit-PatchSet: 3
    Gerrit-Owner: Kent Tamura <tk...@chromium.org>
    Gerrit-Reviewer: Kent Tamura <tk...@chromium.org>
    Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>
    Gerrit-CC: Menard, Alexis <alexis...@intel.com>
    Gerrit-Attention: Kent Tamura <tk...@chromium.org>
    Gerrit-Comment-Date: Tue, 20 Jan 2026 04:49:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Koji Ishii (Gerrit)

    unread,
    Jan 19, 2026, 11:50:19 PM (4 days ago) Jan 19
    to Kent Tamura, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, zol...@webkit.org
    Attention needed from Kent Tamura

    Koji Ishii added 1 comment

    File third_party/blink/renderer/core/style/fit_text.h
    Line 49, Patchset 3 (Latest): std::optional<double> ScaleFactorLimit() const { return scale_factor_limit_; }
    Koji Ishii . unresolved

    nit: Not strong, but I think `float` has enough precision, and can save 4 bytes on `ComputedStyle`. I checked two examples, `Color` and `DynamicRangeLimit`, both stores as floats.

    Gerrit-Comment-Date: Tue, 20 Jan 2026 04:49:56 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Koji Ishii <ko...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Kent Tamura (Gerrit)

    unread,
    Jan 20, 2026, 1:04:28 AM (3 days ago) Jan 20
    to Kent Tamura, Koji Ishii, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, zol...@webkit.org

    Kent Tamura voted and added 1 comment

    Votes added by Kent Tamura

    Commit-Queue+2

    1 comment

    File third_party/blink/renderer/core/style/fit_text.h
    Line 49, Patchset 3: std::optional<double> ScaleFactorLimit() const { return scale_factor_limit_; }
    Koji Ishii . resolved

    nit: Not strong, but I think `float` has enough precision, and can save 4 bytes on `ComputedStyle`. I checked two examples, `Color` and `DynamicRangeLimit`, both stores as floats.

    Koji Ishii

    e.g., https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/resolver/style_builder_converter.cc;l=249?q=ComputePercentage&ss=chromium

    Kent Tamura

    Yeah, `float` should be enough. Thank you for the suggestion.

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I4e9e539d6564f09b7c696c43b077f4c77ff4fde2
      Gerrit-Change-Number: 7495731
      Gerrit-PatchSet: 4
      Gerrit-Owner: Kent Tamura <tk...@chromium.org>
      Gerrit-Reviewer: Kent Tamura <tk...@chromium.org>
      Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>
      Gerrit-CC: Menard, Alexis <alexis...@intel.com>
      Gerrit-Comment-Date: Tue, 20 Jan 2026 06:03:48 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Koji Ishii <ko...@chromium.org>
      satisfied_requirement
      open
      diffy

      Chromium LUCI CQ (Gerrit)

      unread,
      Jan 20, 2026, 1:08:05 AM (3 days ago) Jan 20
      to Kent Tamura, Koji Ishii, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, zol...@webkit.org

      Chromium LUCI CQ submitted the change with unreviewed changes

      Unreviewed changes

      3 is the latest approved patch-set.
      The change was submitted with unreviewed changes in the following files:

      ```
      The name of the file: third_party/blink/renderer/core/style/fit_text.h
      Insertions: 3, Deletions: 3.

      @@ -34,7 +34,7 @@
       
        public:
         FitText() = default;
      -  FitText(FitTextType type, FitTextTarget target, std::optional<double> limit)
      +  FitText(FitTextType type, FitTextTarget target, std::optional<float> limit)
             : type_(type), target_(target), scale_factor_limit_(limit) {}
       
         bool operator==(const FitText& other) const {
      @@ -46,7 +46,7 @@
         FitTextTarget Target() const { return target_; }
         FitTextMethod Method() const { return FitTextMethod::kScale; }
         // This returns 1.0 for "100%".
      -  std::optional<double> ScaleFactorLimit() const { return scale_factor_limit_; }
      +  std::optional<float> ScaleFactorLimit() const { return scale_factor_limit_; }
       
         // A debug helper.
         String ToString() const;
      @@ -54,7 +54,7 @@
        private:
         FitTextType type_ = FitTextType::kNone;
         FitTextTarget target_ = FitTextTarget::kConsistent;
      -  std::optional<double> scale_factor_limit_;
      +  std::optional<float> scale_factor_limit_;
       };
       
       }  // namespace blink
      ```
      ```
      The name of the file: third_party/blink/renderer/core/css/resolver/style_builder_converter.cc
      Insertions: 4, Deletions: 2.

      @@ -4212,8 +4212,10 @@
         std::optional<double> limit;
         if (list.length() > next_index) {
           if (const auto* limit_value =
      -            DynamicTo<CSSNumericLiteralValue>(list.Item(next_index))) {
      -      limit.emplace(limit_value->ComputePercentage() / 100);
      +            DynamicTo<CSSPrimitiveValue>(list.Item(next_index))) {
      +      limit.emplace(limit_value->ComputePercentage<float>(
      +                        state.CssToLengthConversionData()) /
      +                    100);
           }
           ++next_index;
         }
      ```
      ```
      The name of the file: third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
      Insertions: 0, Deletions: 1.

      @@ -9512,7 +9512,6 @@
           list->Append(*target);
         }
       
      -  // TODO(crbug.com/417306102): This should be <percentage>.
         if (CSSValue* percent =
                 ConsumePercent(stream, context, local_context,
                                CSSPrimitiveValue::ValueRange::kNonNegative)) {
      ```
      ```
      The name of the file: third_party/blink/renderer/core/layout/inline/fit_text_utils.cc
      Insertions: 2, Deletions: 3.

      @@ -4,7 +4,6 @@
       
       #include "third_party/blink/renderer/core/layout/inline/fit_text_utils.h"
       
      -#include "base/numerics/safe_conversions.h"
       #include "third_party/blink/renderer/core/frame/local_frame.h"
       #include "third_party/blink/renderer/core/frame/settings.h"
       #include "third_party/blink/renderer/core/inspector/console_message.h"
      @@ -294,7 +293,7 @@
         }
         if (std::isfinite(minimum_scale)) {
           if (fit_text.ScaleFactorLimit().has_value()) {
      -      float limit = base::saturated_cast<float>(*fit_text.ScaleFactorLimit());
      +      float limit = *fit_text.ScaleFactorLimit();
             minimum_scale = fit_text.Type() == FitTextType::kGrow
                                 ? std::min(minimum_scale, std::max(limit, 1.0f))
                                 : std::max(minimum_scale, std::min(limit, 1.0f));
      @@ -360,7 +359,7 @@
         float scale_factor = (container_width - static_total_size).ToFloat() /
                              flexible_total_size.ToFloat();
         if (fit_text.ScaleFactorLimit().has_value()) {
      -    float limit = base::saturated_cast<float>(*fit_text.ScaleFactorLimit());
      +    float limit = *fit_text.ScaleFactorLimit();
           return fit_text.Type() == FitTextType::kGrow
                      ? std::min(scale_factor, std::max(limit, 1.0f))
                      : std::max(scale_factor, std::min(limit, 1.0f));
      ```

      Change information

      Commit message:
      FitText: Update the last component of `text-fit` property

      It should be <percentage>.

      * Accept non-negative percentage values
      * blink::FitText stores std::optional<double>
      * It limits a scaling factor, not font-size.
      Bug: 417306102
      Change-Id: I4e9e539d6564f09b7c696c43b077f4c77ff4fde2
      Commit-Queue: Kent Tamura <tk...@chromium.org>
      Reviewed-by: Koji Ishii <ko...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#1571434}
      Files:
      • M third_party/blink/renderer/core/css/properties/computed_style_utils.cc
      • M third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
      • M third_party/blink/renderer/core/css/resolver/style_builder_converter.cc
      • M third_party/blink/renderer/core/layout/inline/fit_text_utils.cc
      • M third_party/blink/renderer/core/style/fit_text.cc
      • M third_party/blink/renderer/core/style/fit_text.h
      • M third_party/blink/web_tests/wpt_internal/css/fit-width-text/text-grow-computed.html
      • M third_party/blink/web_tests/wpt_internal/css/fit-width-text/text-grow-invalid.html
      • M third_party/blink/web_tests/wpt_internal/css/fit-width-text/text-grow-valid.html
      • M third_party/blink/web_tests/wpt_internal/css/fit-width-text/text-shrink-per-line-scale-minimum-setting.html
      • M third_party/blink/web_tests/wpt_internal/css/fit-width-text/text-shrink-per-line-scale.html
      Change size: M
      Delta: 11 files changed, 65 insertions(+), 55 deletions(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Koji Ishii
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I4e9e539d6564f09b7c696c43b077f4c77ff4fde2
      Gerrit-Change-Number: 7495731
      Gerrit-PatchSet: 5
      Gerrit-Owner: Kent Tamura <tk...@chromium.org>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Kent Tamura <tk...@chromium.org>
      Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages