This implements fit-content() for basic layout support. For intrinsic
contribution, we will do that in the following patches.
Details
Details
- Reviewers
TYLin emilio - Group Reviewers
layout-reviewers - Commits
- rMOZILLACENTRALc5389bf6f509: Bug 1312588 - Part 3: Implement the layout part of fit-content() without the…
- Bugzilla Bug ID
- 1312588
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable - Build Status
Buildable 327174 Build 421171: arc lint + arc unit
Event Timeline
Comment Actions
Code analysis found 1 defect in the diff 430608:
- 1 defect found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Code analysis found 1 defect in the diff 432562:
- 1 defect found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Code analysis found 1 defect in the diff 433170:
- 1 defect found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
| layout/generic/nsIFrame.cpp | ||
|---|---|---|
| 6588 | nscoord arg = aFitContentArg.valueOrFrom([&] {
return aContainingBlockSize.ISize() - ...;
}); | |
| layout/generic/nsIFrame.h | ||
| 4856 | I'd probably factor this into a separate variable, just for readability: Maybe<nscoord> fitContentArg;
if (aSize.IsFitContentFunc()) {
fitContentArg.emplace(ComputeISizeValue(aWM, aContainingBlockSize, aContentEdgeToBoxSizing, aSize.AsFitContentFunc());
}
return ComputeISizeValue(...); | |
Comment Actions
Code analysis found 1 defect in the diff 434418:
- 1 defect found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
| layout/generic/nsIFrame.cpp | ||
|---|---|---|
| 6585 | aContainingBlockSize.ISize(aWM) can be replaced by something like aAvailableISizeOverride ? *aAvailableISizeOverride : aContainingBlockSize.ISize(aWM) | |
| layout/generic/nsIFrame.h | ||
| 4817 | I don't feel aFitContentArg is a good name because the <length-percentage> provided by fit-content() function is to override the available size from the containing block, i.e. aContainingBlockSize.ISize(). https://drafts.csswg.org/css-sizing-3/#available Even so, it's still unclear whether this available size override has already considered box-sizing or not. I'll prefer this doesn't consider box-sizing at all because we all do it in this function anyway. How about we call this aAvailableISizeOverride, and add a document like the following ? @param aAvailableISizeOverride If this has a value, it is used as the available inline-size instead of aContainingBlockSize.ISize(aWM) when resolving fit-content. (Please double check if the suggestion wording is correct.) | |
| 4845–4852 | If you agree the box-sizing should be handle in ComputeISizeValue in my other inline comment, this can be Maybe<nscoord> availbleISizeOverride;
if (aSize.IsFitContentFunction()) {
availbleISizeOverride.emplace(aSize.AsFitContentFunction().Resolve(
aContainingBlockSize.ISize(aWM)));
} | |
| layout/generic/nsIFrame.h | ||
|---|---|---|
| 4817 | This makes more sense. Yes, let's move the handle of box-sizing into this function to avoid the potential incorrect calculation. | |
Comment Actions
Code analysis found 1 defect in the diff 448216:
- 1 defect found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.