Bug 240388 - computeIntrinsicLogicalContentHeightUsing doesn't take intrinsicBorderForFieldset() into account
Summary: computeIntrinsicLogicalContentHeightUsing doesn't take intrinsicBorderForFiel...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on:
Blocks: 240068
  Show dependency treegraph
 
Reported: 2022-05-13 09:50 PDT by Oriol Brufau
Modified: 2022-05-16 16:05 PDT (History)
10 users (show)

See Also:


Attachments
Patch (10.79 KB, patch)
2022-05-13 11:20 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2022-05-13 09:50:04 PDT
Runt this:

  <fieldset style="height: min-content; border: solid cyan">
    <legend>foo</legend>
    <div style="border: solid fuchsia">bar</div>
  </fieldset>

The fieldset is not big enough, the contents overflow.

Or run this:

  <fieldset id="target" style="height: min-content">
    <legend>foo</legend>
  </fieldset>
  <script>
  console.log(getComputedStyle(target).height);
  </script>

It logs -15.59375px, negative height!

That's because RenderBox::computeIntrinsicLogicalContentHeightUsing uses

        if (intrinsicContentHeight && style().boxSizing() == BoxSizing::BorderBox)
            return intrinsicContentHeight.value() + borderAndPaddingLogicalHeight();
        return intrinsicContentHeight;

This logic should be moved into a virtual method, and override it in RenderBlock to take intrinsicBorderForFieldset() into account.
Just like adjustBorderBoxLogicalHeightForBoxSizing and adjustIntrinsicLogicalHeightForBoxSizing.
Comment 1 Oriol Brufau 2022-05-13 11:20:50 PDT
Created attachment 459312 [details]
Patch
Comment 2 EWS Watchlist 2022-05-13 11:24:17 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Darin Adler 2022-05-16 13:04:11 PDT
Comment on attachment 459312 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:3269
>          return intrinsicContentHeight;

After the refactoring, this could just be return std::nullopt, and I think it’s clearer that way.
Comment 4 EWS 2022-05-16 16:04:39 PDT
Committed r294275 (250621@main): <https://commits.webkit.org/250621@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459312 [details].
Comment 5 Radar WebKit Bug Importer 2022-05-16 16:05:18 PDT
<rdar://problem/93378304>