serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Browser: Fix calculation of greatest child width

Open NoahR02 opened this issue 2 years ago • 2 comments

The greatest child width in BlockFormattingContext.cpp does not account for the min-width of a child.

You can reproduce this issue with the following html document:

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
  <style>
    .debug_here {
      display: inline-block;
      border: 1px solid red;
    }

    .AvatarStack {
      min-width: 26px;
      min-height: 50px;
    }

    .AvatarStack .AvatarStack-body {
      position: absolute;
      width: 24px;
      height: 24px;
      opacity: 0.3;
      background: red;
    }

    .text {
      display: inline-block;
    }
  </style>
</head>

<body>
  <div class="debug_here">
    <div class="AvatarStack">
      <div class="AvatarStack-body"></div>
    </div>
  </div>
  <div class="text"><a>awesomekling</a><a>LibJS:Only use 1 bit for Cell boolean flag</a></div>
</body>

</html>

If you dump the layout blocks, then you will see that debug_here has a width of 0px. debug_here should be the greatest width of it's child.

NoahR02 avatar Oct 26 '22 11:10 NoahR02

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

BuggieBot avatar Oct 29 '22 23:10 BuggieBot

In InlineFormattingContext.cpp, if the width is auto then we calculate the shrink fit which in turn calls greatest_child_width. The available space we provide in this scenario is under a intrinsic sizing constraint, so compute_width never has the opportunity to calculate the width correctly because it does not account for min or max width which in turn messes up the calculation of the calling InlineFormattingContext.

We cannot accurately calculate the shrink fit correctly if we return early from BlockFormattingContext::compute_width. If we remove the if statement then the shrink fit calculation is now correct and produces the correct width: image

Test Case:

<!DOCTYPE html><html><head><style>    
    .debug_here {    
      display: inline-block;    
      border: 1px solid red;   
    }    
    .AvatarStack {    
      min-width: 26px;    
      min-height: 50px;    
    }    
</style></head><body><div class="debug_here"><div class="AvatarStack">

NoahR02 avatar Oct 30 '22 20:10 NoahR02

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Nov 22 '22 16:11 stale[bot]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

stale[bot] avatar Nov 30 '22 06:11 stale[bot]