Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upStart supporting images in layout 2020 #24928
Conversation
highfive
commented
Nov 29, 2019
|
Heads up! This PR modifies the following files:
|
highfive
commented
Nov 29, 2019
| match (computed_margin_inline_start, computed_margin_inline_end) { | ||
| (LengthOrAuto::Auto, LengthOrAuto::Auto) => (inline_margins / 2., inline_margins / 2.), | ||
| (LengthOrAuto::Auto, LengthOrAuto::LengthPercentage(end)) => (inline_margins - end, end), | ||
| (LengthOrAuto::LengthPercentage(start), _) => (start, inline_margins - start), |
This comment has been minimized.
This comment has been minimized.
SimonSapin
Nov 29, 2019
Member
Nit: This _ can only be Auto, right? Could we make it a specific pattern?
This comment has been minimized.
This comment has been minimized.
nox
Nov 29, 2019
Author
Member
It can be LengthOrAuto::LengthPercentage too, which we ignore because that's over-constraining.
| ) -> IndependentLayout { | ||
| let (fragments, content_block_size) = match self.kind { | ||
| ReplacedContentKind::Image(ref image) => { | ||
| // FIXME(nox): We should not assume block size is known. |
This comment has been minimized.
This comment has been minimized.
SimonSapin
Nov 29, 2019
Member
Or, on the contrary, isn’t it always known for replaced boxes and should we take DefiniteContainingBlock here instead?
This comment has been minimized.
This comment has been minimized.
nox
Nov 29, 2019
Author
Member
You are right, at that point we should know both sizes, I'll make it use DefiniteContainingBlock.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nox
Nov 30, 2019
Author
Member
It turns out we can't do that: this is also called from IndependentFormattingContext::layout, where we don't know for sure the block size.
This comment has been minimized.
This comment has been minimized.
nox
Nov 30, 2019
Author
Member
Tbh that makes me wonder if we shouldn't separate replaced content from independent formatting contexts.
This comment has been minimized.
This comment has been minimized.
SimonSapin
Nov 30, 2019
Member
Yeah I considered that when reading your previous message. Or if not separate data types, at least remove IndependentFormattingContext::layout and make parent boxes call IndependentFormattingContext::as_replaced themselves.
This comment has been minimized.
This comment has been minimized.
|
@bors-servo r+ |
|
|
Start supporting images in layout 2020
|
|
|
One failure in layout 2020 mode, could be legit?
|
|
The fixed crash is due to removing on @bors-servo r+ |
|
|
Start supporting images in layout 2020
|
|
|
@bors-servo retry #24726 |
Start supporting images in layout 2020
|
|
|
@bors-servo retry #24622 |
Start supporting images in layout 2020
|
|
nox commentedNov 29, 2019
No description provided.