Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layout `position: fixed` in the initial containing block #25273

Merged
merged 13 commits into from Dec 13, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Replace the closure in `layout_in_flow_non_replaced_block_level` with…

… an enum
  • Loading branch information
SimonSapin committed Dec 13, 2019
commit c0962aa3fcadce403df4e73c154b9a87fcb4d6c1
@@ -7,7 +7,7 @@
use crate::context::LayoutContext;
use crate::flow::float::{FloatBox, FloatContext};
use crate::flow::inline::InlineFormattingContext;
use crate::formatting_contexts::{IndependentFormattingContext, IndependentLayout};
use crate::formatting_contexts::{IndependentFormattingContext, IndependentLayout, NonReplacedIFC};
use crate::fragments::{AnonymousFragment, BoxFragment, Fragment};
use crate::fragments::{CollapsedBlockMargins, CollapsedMargin};
use crate::geom::flow_relative::{Rect, Sides, Vec2};
@@ -274,19 +274,9 @@ impl BlockLevelBox {
positioning_context,
containing_block,
style,
BlockLevelKind::SameFormattingContextBlock,
|positioning_context,
containing_block,
collapsible_with_parent_start_margin| {
contents.layout(
layout_context,
positioning_context,
containing_block,
tree_rank,
float_context,
collapsible_with_parent_start_margin,
)
},
BlockLevelKind::SameFormattingContextBlock(contents),
tree_rank,
float_context
))
},
BlockLevelBox::Independent(contents) => match contents.as_replaced() {
@@ -300,20 +290,9 @@ impl BlockLevelBox {
positioning_context,
containing_block,
&contents.style,
BlockLevelKind::EstablishesAnIndependentFormattingContext,
|positioning_context, containing_block, _| {
let independent_layout = non_replaced.layout(
layout_context,
positioning_context,
containing_block,
tree_rank,
);
FlowLayout {
fragments: independent_layout.fragments,
content_block_size: independent_layout.content_block_size,
collapsible_margins_in_children: CollapsedBlockMargins::zero(),
}
},
BlockLevelKind::EstablishesAnIndependentFormattingContext(non_replaced),
tree_rank,
float_context
)),
},
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(box_) => {
@@ -332,10 +311,9 @@ impl BlockLevelBox {
}
}

#[derive(PartialEq)]
enum BlockLevelKind {
SameFormattingContextBlock,
EstablishesAnIndependentFormattingContext,
enum BlockLevelKind<'a> {
This conversation was marked as resolved by nox

This comment has been minimized.

Copy link
@nox

nox Dec 13, 2019

Member

Would it make more sense to name that BlockKind?

This comment has been minimized.

Copy link
@nox

nox Dec 13, 2019

Member

Or BlockContents, or something with "contents" somewhere. I don't like how it has less variants than BlockLevelBox.

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Dec 13, 2019

Author Member

Done.

SameFormattingContextBlock(&'a BlockContainer),
EstablishesAnIndependentFormattingContext(NonReplacedIFC<'a>),
}

/// https://drafts.csswg.org/css2/visudet.html#blockwidth
@@ -345,12 +323,9 @@ fn layout_in_flow_non_replaced_block_level<'a>(
positioning_context: &mut PositioningContext<'a>,
containing_block: &ContainingBlock,
style: &Arc<ComputedValues>,
block_level_kind: BlockLevelKind,
layout_contents: impl FnOnce(
&mut PositioningContext<'a>,
&ContainingBlock,
CollapsibleWithParentStartMargin,
) -> FlowLayout,
block_level_kind: BlockLevelKind<'a>,
tree_rank: usize,
float_context: Option<&mut FloatContext>,
) -> BoxFragment {
let cbis = containing_block.inline_size;
let padding = style.padding().percentages_relative_to(cbis);
@@ -424,20 +399,38 @@ fn layout_in_flow_non_replaced_block_level<'a>(
);

let this_start_margin_can_collapse_with_children = CollapsibleWithParentStartMargin(
block_level_kind == BlockLevelKind::SameFormattingContextBlock &&
matches!(block_level_kind, BlockLevelKind::SameFormattingContextBlock(_)) &&
This conversation was marked as resolved by nox

This comment has been minimized.

Copy link
@nox

nox Dec 13, 2019

Member

Maybe a boolean method for this.

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Dec 13, 2019

Author Member

This is removed in a latter commit.

pb.block_start == Length::zero(),
);
let this_end_margin_can_collapse_with_children = block_size == LengthOrAuto::Auto &&
min_box_size.block == Length::zero() &&
pb.block_end == Length::zero() &&
block_level_kind == BlockLevelKind::SameFormattingContextBlock;
matches!(block_level_kind, BlockLevelKind::SameFormattingContextBlock(_));

positioning_context.for_maybe_position_relative(layout_context, style, |positioning_context| {
This conversation was marked as resolved by nox

This comment has been minimized.

Copy link
@nox

nox Dec 13, 2019

Member

I understand the logic but I'm not sure I like how big this closure is and how it drifts everything even more to the right. Do you think it could be its own function, living in the module next to this one?

let mut flow_layout = layout_contents(
positioning_context,
&containing_block_for_children,
this_start_margin_can_collapse_with_children,
);
let mut flow_layout = match block_level_kind {
BlockLevelKind::SameFormattingContextBlock(contents) => contents.layout(
layout_context,
positioning_context,
&containing_block_for_children,
tree_rank,
float_context,
this_start_margin_can_collapse_with_children,
),
BlockLevelKind::EstablishesAnIndependentFormattingContext(non_replaced) => {
let independent_layout = non_replaced.layout(
layout_context,
positioning_context,
&containing_block_for_children,
tree_rank,
);
FlowLayout {
fragments: independent_layout.fragments,
content_block_size: independent_layout.content_block_size,
collapsible_margins_in_children: CollapsedBlockMargins::zero(),
}
}
};
let mut block_margins_collapsed_with_children = CollapsedBlockMargins::from_margin(&margin);
if this_start_margin_can_collapse_with_children.0 {
block_margins_collapsed_with_children
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.