From ee8741b7a8d7313a3ad1ea6ef81cebf7cb262891 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 13 Aug 2015 09:49:40 -0700 Subject: [PATCH] layout: Fix several bugs relating to inline borders, padding, and margins. * The code that attempted to strip out borders that span multiple fragments in the same element could go wrong if fragments were stripped out due to text clumping or whitespace stripping. This patch rewrites that code to maintain flags in the inline fragment context specifying whether the node is the beginning or end of the element. Not only is this easier to maintain, it's closer in spirit to what roc originally suggested two years ago: it's isomorphic to "begin element, end element" markers for inline layout. * Padding and margins for spans containing inline-blocks are now properly handled via a division of labor between the `InlineBlock` fragment and the `BlockFlow` that represents the inline-block. * Unscanned text fragments may not be joined together into a text run if borders, padding, or margins separate them. Because Servo now matches the rendering of Gecko and WebKit on the `input_button_margins_a` reftest, I had to modify it to add some vertical alignment. The combined effect of all of these fixes places "Advertising" on the right place on google.com. --- components/layout/construct.rs | 85 +++--- components/layout/display_list_builder.rs | 13 +- components/layout/fragment.rs | 244 +++++++++++++----- components/layout/inline.rs | 18 +- components/layout/text.rs | 14 +- components/style/properties.mako.rs | 44 ++-- tests/ref/basic.list | 1 + tests/ref/input_button_margins_a.html | 2 + tests/ref/input_button_margins_ref.html | 1 + tests/ref/margin_padding_inline_block_a.html | 10 + .../ref/margin_padding_inline_block_ref.html | 17 ++ 11 files changed, 319 insertions(+), 130 deletions(-) create mode 100644 tests/ref/margin_padding_inline_block_a.html create mode 100644 tests/ref/margin_padding_inline_block_ref.html diff --git a/components/layout/construct.rs b/components/layout/construct.rs index d701edfc435..eb43c81b93e 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -27,7 +27,8 @@ use fragment::{InlineAbsoluteHypotheticalFragmentInfo, TableColumnFragmentInfo}; use fragment::{InlineBlockFragmentInfo, SpecificFragmentInfo, UnscannedTextFragmentInfo}; use fragment::{WhitespaceStrippingResult}; use incremental::{RECONSTRUCT_FLOW, RestyleDamage}; -use inline::{InlineFlow, InlineFragmentNodeInfo}; +use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFlow, InlineFragmentNodeFlags}; +use inline::{InlineFragmentNodeInfo, LAST_FRAGMENT_OF_ELEMENT}; use list_item::{ListItemFlow, ListStyleTypeContent}; use multicol::MulticolFlow; use parallel; @@ -217,20 +218,7 @@ impl InlineFragmentsAccumulator { address: node.opaque(), pseudo: node.get_pseudo_element_type().strip(), style: node.style().clone(), - }), - bidi_control_chars: None, - restyle_damage: node.restyle_damage(), - } - } - - fn from_inline_node_and_style(node: &ThreadSafeLayoutNode, style: Arc) - -> InlineFragmentsAccumulator { - InlineFragmentsAccumulator { - fragments: IntermediateInlineFragments::new(), - enclosing_node: Some(InlineFragmentNodeInfo { - address: node.opaque(), - pseudo: node.get_pseudo_element_type().strip(), - style: style, + flags: InlineFragmentNodeFlags::empty(), }), bidi_control_chars: None, restyle_damage: node.restyle_damage(), @@ -254,15 +242,16 @@ impl InlineFragmentsAccumulator { restyle_damage, } = self; if let Some(enclosing_node) = enclosing_node { - let frag_len = fragments.fragments.len(); - for (idx, frag) in fragments.fragments.iter_mut().enumerate() { - - // frag is first inline fragment in the inline node - let is_first = idx == 0; - // frag is the last inline fragment in the inline node - let is_last = idx == frag_len - 1; - - frag.add_inline_context_style(enclosing_node.clone(), is_first, is_last); + let fragment_count = fragments.fragments.len(); + for (index, fragment) in fragments.fragments.iter_mut().enumerate() { + let mut enclosing_node = enclosing_node.clone(); + if index == 0 { + enclosing_node.flags.insert(FIRST_FRAGMENT_OF_ELEMENT) + } + if index == fragment_count - 1 { + enclosing_node.flags.insert(LAST_FRAGMENT_OF_ELEMENT) + } + fragment.add_inline_context_style(enclosing_node); } // Control characters are later discarded in transform_text, so they don't affect the @@ -596,6 +585,7 @@ impl<'a> FlowConstructor<'a> { let fragment_info = SpecificFragmentInfo::UnscannedText( UnscannedTextFragmentInfo::from_text(" ".to_owned())); properties::modify_style_for_replaced_content(&mut whitespace_style); + properties::modify_style_for_text(&mut whitespace_style); let fragment = Fragment::from_opaque_node_and_style(whitespace_node, whitespace_pseudo, whitespace_style, @@ -863,6 +853,7 @@ impl<'a> FlowConstructor<'a> { let fragment_info = SpecificFragmentInfo::UnscannedText( UnscannedTextFragmentInfo::from_text(" ".to_owned())); properties::modify_style_for_replaced_content(&mut whitespace_style); + properties::modify_style_for_text(&mut whitespace_style); let fragment = Fragment::from_opaque_node_and_style(whitespace_node, whitespace_pseudo, whitespace_style, @@ -966,8 +957,7 @@ impl<'a> FlowConstructor<'a> { node.restyle_damage(), fragment_info); - let mut fragment_accumulator = - InlineFragmentsAccumulator::from_inline_node_and_style(node, modified_style); + let mut fragment_accumulator = InlineFragmentsAccumulator::new(); fragment_accumulator.fragments.fragments.push_back(fragment); fragment_accumulator.fragments.absolute_descendants.push_descendants(abs_descendants); @@ -1718,12 +1708,36 @@ pub fn strip_ignorable_whitespace_from_start(this: &mut LinkedList) { this.pop_front().unwrap()) } WhitespaceStrippingResult::FragmentContainedOnlyWhitespace => { - this.pop_front(); + let removed_fragment = this.pop_front().unwrap(); + if let Some(ref mut remaining_fragment) = this.front_mut() { + if let Some(ref mut inline_context_of_remaining_fragment) = + remaining_fragment.inline_context { + if let Some(ref inline_context_of_removed_fragment) = + removed_fragment.inline_context { + for (i, inline_context_node_from_removed_fragment) in + inline_context_of_removed_fragment.nodes.iter().enumerate() { + if i >= inline_context_of_remaining_fragment.nodes.len() { + break + } + if !inline_context_node_from_removed_fragment.flags.contains( + FIRST_FRAGMENT_OF_ELEMENT) { + continue + } + if inline_context_node_from_removed_fragment.address != + inline_context_of_remaining_fragment.nodes[i].address { + continue + } + inline_context_of_remaining_fragment.nodes[i].flags.insert( + FIRST_FRAGMENT_OF_ELEMENT); + } + } + } + } } } } linked_list::prepend_from(this, - &mut leading_fragments_consisting_of_solely_bidi_control_characters) + &mut leading_fragments_consisting_of_solely_bidi_control_characters); } /// Strips ignorable whitespace from the end of a list of fragments. @@ -1741,7 +1755,10 @@ pub fn strip_ignorable_whitespace_from_end(this: &mut LinkedList) { this.pop_back().unwrap()) } WhitespaceStrippingResult::FragmentContainedOnlyWhitespace => { - this.pop_back(); + let removed_fragment = this.pop_back().unwrap(); + if let Some(ref mut remaining_fragment) = this.back_mut() { + remaining_fragment.meld_with_next_inline_fragment(&removed_fragment); + } } } } @@ -1772,13 +1789,17 @@ fn bidi_control_chars(style: &Arc) -> Option<(&'static str, &'st } } -fn control_chars_to_fragment(node: &InlineFragmentNodeInfo, text: &str, - restyle_damage: RestyleDamage) -> Fragment { +fn control_chars_to_fragment(node: &InlineFragmentNodeInfo, + text: &str, + restyle_damage: RestyleDamage) + -> Fragment { let info = SpecificFragmentInfo::UnscannedText( UnscannedTextFragmentInfo::from_text(String::from(text))); + let mut style = node.style.clone(); + properties::modify_style_for_text(&mut style); Fragment::from_opaque_node_and_style(node.address, node.pseudo, - node.style.clone(), + style, restyle_damage, info) } diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index d2dd57f0e21..9e82add3a12 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -18,7 +18,7 @@ use flow::{self, BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED}; use flow_ref; use fragment::{CoordinateSystem, Fragment, HAS_LAYER, IframeFragmentInfo, ImageFragmentInfo}; use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo}; -use inline::InlineFlow; +use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFlow, LAST_FRAGMENT_OF_ELEMENT}; use list_item::ListItemFlow; use model::{self, MaybeAuto, ToGfxMatrix}; use table_cell::CollapsedBordersForCell; @@ -51,8 +51,8 @@ use style::computed_values::{background_attachment, background_clip, background_ use style::computed_values::{background_repeat, background_size}; use style::computed_values::{border_style, image_rendering, overflow_x, position}; use style::computed_values::{visibility, transform, transform_style}; -use style::properties::ComputedValues; use style::properties::style_structs::Border; +use style::properties::{self, ComputedValues}; use style::values::RGBA; use style::values::computed; use style::values::computed::LinearGradient; @@ -951,13 +951,20 @@ impl FragmentDisplayListBuilding for Fragment { level, &stacking_relative_border_box, &clip); + + let mut style = node.style.clone(); + properties::modify_border_style_for_inline_sides( + &mut style, + node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT), + node.flags.contains(LAST_FRAGMENT_OF_ELEMENT)); self.build_display_list_for_borders_if_applicable( - &*node.style, + &*style, border_painting_mode, display_list, &stacking_relative_border_box, level, &clip); + self.build_display_list_for_outline_if_applicable( &*node.style, display_list, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index f03e0cc7f7a..c11d9170679 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -13,7 +13,8 @@ use flow; use flow::Flow; use flow_ref::{self, FlowRef}; use incremental::{self, RestyleDamage}; -use inline::{InlineFragmentContext, InlineFragmentNodeInfo, InlineMetrics}; +use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFragmentContext, InlineFragmentNodeInfo}; +use inline::{InlineMetrics, LAST_FRAGMENT_OF_ELEMENT}; use layout_debug; use model::{self, IntrinsicISizes, IntrinsicISizesContribution, MaybeAuto, specified}; use text; @@ -39,7 +40,7 @@ use style::computed_values::content::ContentItem; use style::computed_values::{border_collapse, clear, mix_blend_mode, overflow_wrap, overflow_x}; use style::computed_values::{position, text_align, text_decoration, transform_style, white_space}; use style::computed_values::{word_break, z_index}; -use style::properties::{self, ComputedValues}; +use style::properties::ComputedValues; use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto}; use style::values::computed::{LengthOrPercentageOrNone}; use text::TextRunScanner; @@ -194,7 +195,9 @@ impl SpecificFragmentInfo { SpecificFragmentInfo::Iframe(_) => "SpecificFragmentInfo::Iframe", SpecificFragmentInfo::Image(_) => "SpecificFragmentInfo::Image", SpecificFragmentInfo::InlineAbsolute(_) => "SpecificFragmentInfo::InlineAbsolute", - SpecificFragmentInfo::InlineAbsoluteHypothetical(_) => "SpecificFragmentInfo::InlineAbsoluteHypothetical", + SpecificFragmentInfo::InlineAbsoluteHypothetical(_) => { + "SpecificFragmentInfo::InlineAbsoluteHypothetical" + } SpecificFragmentInfo::InlineBlock(_) => "SpecificFragmentInfo::InlineBlock", SpecificFragmentInfo::ScannedText(_) => "SpecificFragmentInfo::ScannedText", SpecificFragmentInfo::Table => "SpecificFragmentInfo::Table", @@ -876,16 +879,10 @@ impl Fragment { /// Adds a style to the inline context for this fragment. If the inline context doesn't exist /// yet, it will be created. - pub fn add_inline_context_style(&mut self, - mut node_info: InlineFragmentNodeInfo, - first_frag: bool, - last_frag: bool) { + pub fn add_inline_context_style(&mut self, node_info: InlineFragmentNodeInfo) { if self.inline_context.is_none() { self.inline_context = Some(InlineFragmentContext::new()); } - if !first_frag || !last_frag { - properties::modify_style_for_inline_sides(&mut node_info.style, first_frag, last_frag) - }; self.inline_context.as_mut().unwrap().nodes.push(node_info); } @@ -1015,17 +1012,24 @@ impl Fragment { #[inline] fn border_width(&self) -> LogicalMargin { let style_border_width = match self.specific { - SpecificFragmentInfo::ScannedText(_) => LogicalMargin::zero(self.style.writing_mode), + SpecificFragmentInfo::ScannedText(_) | + SpecificFragmentInfo::InlineBlock(_) => LogicalMargin::zero(self.style.writing_mode), _ => self.style().logical_border_width(), }; match self.inline_context { None => style_border_width, Some(ref inline_fragment_context) => { - inline_fragment_context.nodes - .iter() - .fold(style_border_width, - |acc, node| acc + node.style.logical_border_width()) + inline_fragment_context.nodes.iter().fold(style_border_width, |accumulator, node| { + let mut this_border_width = node.style.logical_border_width(); + if !node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT) { + this_border_width.inline_start = Au(0) + } + if !node.flags.contains(LAST_FRAGMENT_OF_ELEMENT) { + this_border_width.inline_end = Au(0) + } + accumulator + this_border_width + }) } } } @@ -1037,7 +1041,6 @@ impl Fragment { /// (for example, via constraint solving for blocks). pub fn compute_inline_direction_margins(&mut self, containing_block_inline_size: Au) { match self.specific { - SpecificFragmentInfo::InlineBlock(_) | SpecificFragmentInfo::Table | SpecificFragmentInfo::TableCell | SpecificFragmentInfo::TableRow | @@ -1046,27 +1049,41 @@ impl Fragment { self.margin.inline_end = Au(0); return } - _ => {} + SpecificFragmentInfo::InlineBlock(_) => { + // Inline-blocks do not take self margins into account but do account for margins + // from outer inline contexts. + self.margin.inline_start = Au(0); + self.margin.inline_end = Au(0); + } + _ => { + let margin = self.style().logical_margin(); + self.margin.inline_start = + MaybeAuto::from_style(margin.inline_start, + containing_block_inline_size).specified_or_zero(); + self.margin.inline_end = + MaybeAuto::from_style(margin.inline_end, + containing_block_inline_size).specified_or_zero(); + } } - - let margin = self.style().logical_margin(); - self.margin.inline_start = - MaybeAuto::from_style(margin.inline_start, - containing_block_inline_size).specified_or_zero(); - self.margin.inline_end = - MaybeAuto::from_style(margin.inline_end, - containing_block_inline_size).specified_or_zero(); - if let Some(ref inline_context) = self.inline_context { for node in &inline_context.nodes { let margin = node.style.logical_margin(); - self.margin.inline_start = self.margin.inline_start + + let this_inline_start_margin = if !node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT) { + Au(0) + } else { MaybeAuto::from_style(margin.inline_start, - containing_block_inline_size).specified_or_zero(); - self.margin.inline_end = self.margin.inline_end + + containing_block_inline_size).specified_or_zero() + }; + let this_inline_end_margin = if !node.flags.contains(LAST_FRAGMENT_OF_ELEMENT) { + Au(0) + } else { MaybeAuto::from_style(margin.inline_end, - containing_block_inline_size).specified_or_zero(); + containing_block_inline_size).specified_or_zero() + }; + + self.margin.inline_start = self.margin.inline_start + this_inline_start_margin; + self.margin.inline_end = self.margin.inline_end + this_inline_end_margin; } } } @@ -1114,33 +1131,42 @@ impl Fragment { border_collapse::T::collapse => LogicalMargin::zero(self.style.writing_mode), }; - // Compute padding. - let padding = match self.specific { - SpecificFragmentInfo::TableColumn(_) | SpecificFragmentInfo::TableRow | - SpecificFragmentInfo::TableWrapper => LogicalMargin::zero(self.style.writing_mode), - _ => { - let style_padding = match self.specific { - SpecificFragmentInfo::ScannedText(_) => { - LogicalMargin::zero(self.style.writing_mode) - } - _ => model::padding_from_style(self.style(), containing_block_inline_size), - }; + // Compute padding from the fragment's style. + // + // This is zero in the case of `inline-block` because that padding is applied to the + // wrapped block, not the fragment. + let padding_from_style = match self.specific { + SpecificFragmentInfo::TableColumn(_) | + SpecificFragmentInfo::TableRow | + SpecificFragmentInfo::TableWrapper | + SpecificFragmentInfo::InlineBlock(_) => LogicalMargin::zero(self.style.writing_mode), + _ => model::padding_from_style(self.style(), containing_block_inline_size), + }; - match self.inline_context { - None => style_padding, - Some(ref inline_fragment_context) => { - inline_fragment_context.nodes - .iter() - .fold(style_padding, |acc, node| { - acc + model::padding_from_style(&*node.style, - Au(0)) - }) + // Compute padding from the inline fragment context. + let padding_from_inline_fragment_context = match (&self.specific, &self.inline_context) { + (_, &None) | + (&SpecificFragmentInfo::TableColumn(_), _) | + (&SpecificFragmentInfo::TableRow, _) | + (&SpecificFragmentInfo::TableWrapper, _) => { + LogicalMargin::zero(self.style.writing_mode) + } + (_, &Some(ref inline_fragment_context)) => { + let zero_padding = LogicalMargin::zero(self.style.writing_mode); + inline_fragment_context.nodes.iter().fold(zero_padding, |accumulator, node| { + let mut padding = model::padding_from_style(&*node.style, Au(0)); + if !node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT) { + padding.inline_start = Au(0) } - } + if !node.flags.contains(LAST_FRAGMENT_OF_ELEMENT) { + padding.inline_end = Au(0) + } + accumulator + padding + }) } }; - self.border_padding = border + padding + self.border_padding = border + padding_from_style + padding_from_inline_fragment_context } // Return offset from original position because of `position: relative`. @@ -1335,13 +1361,25 @@ impl Fragment { if self.is_primary_fragment() { if let Some(ref context) = self.inline_context { for node in &context.nodes { - let border_width = node.style.logical_border_width().inline_start_end(); - let padding_inline_size = - model::padding_from_style(&*node.style, Au(0)).inline_start_end(); - let margin_inline_size = - model::specified_margin_from_style(&*node.style).inline_start_end(); - result.surrounding_size = result.surrounding_size + border_width + - padding_inline_size + margin_inline_size; + let mut border_width = node.style.logical_border_width(); + let mut padding = model::padding_from_style(&*node.style, Au(0)); + let mut margin = model::specified_margin_from_style(&*node.style); + if !node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT) { + border_width.inline_start = Au(0); + padding.inline_start = Au(0); + margin.inline_start = Au(0); + } + if !node.flags.contains(LAST_FRAGMENT_OF_ELEMENT) { + border_width.inline_end = Au(0); + padding.inline_end = Au(0); + margin.inline_end = Au(0); + } + + result.surrounding_size = + result.surrounding_size + + border_width.inline_start_end() + + padding.inline_start_end() + + margin.inline_start_end(); } } } @@ -1610,11 +1648,14 @@ impl Fragment { this_info.requires_line_break_afterward_if_wrapping_on_newlines = this_info.requires_line_break_afterward_if_wrapping_on_newlines || other_info.requires_line_break_afterward_if_wrapping_on_newlines; + self.border_padding.inline_end = next_fragment.border_padding.inline_end; self.border_box.size.inline = this_info.content_size.inline + self.border_padding.inline_start_end(); } _ => panic!("Can only merge two scanned-text fragments!"), } + + self.meld_with_next_inline_fragment(&next_fragment); } /// Returns true if this fragment is an unscanned text fragment that consists entirely of @@ -1871,17 +1912,67 @@ impl Fragment { } } - /// Returns true if this fragment can merge with another adjacent fragment or false otherwise. + /// Returns true if this fragment can merge with another immediately-following fragment or + /// false otherwise. pub fn can_merge_with_fragment(&self, other: &Fragment) -> bool { match (&self.specific, &other.specific) { (&SpecificFragmentInfo::UnscannedText(ref first_unscanned_text), &SpecificFragmentInfo::UnscannedText(_)) => { // FIXME: Should probably use a whitelist of styles that can safely differ (#3165) + if self.style().get_font() != other.style().get_font() || + self.text_decoration() != other.text_decoration() || + self.white_space() != other.white_space() { + return false + } + let length = first_unscanned_text.text.len(); - self.style().get_font() == other.style().get_font() && - self.text_decoration() == other.text_decoration() && - self.white_space() == other.white_space() && - (length == 0 || first_unscanned_text.text.char_at_reverse(length) != '\n') + if length != 0 && first_unscanned_text.text.char_at_reverse(length) == '\n' { + return false + } + + // If this node has any styles that have border/padding/margins on the following + // side, then we can't merge with the next fragment. + if let Some(ref inline_context) = self.inline_context { + for inline_context_node in inline_context.nodes.iter() { + if !inline_context_node.flags.contains(LAST_FRAGMENT_OF_ELEMENT) { + continue + } + if inline_context_node.style.logical_margin().inline_end != + LengthOrPercentageOrAuto::Length(Au(0)) { + return false + } + if inline_context_node.style.logical_padding().inline_end != + LengthOrPercentage::Length(Au(0)) { + return false + } + if inline_context_node.style.logical_border_width().inline_end != Au(0) { + return false + } + } + } + + // If the next fragment has any styles that have border/padding/margins on the + // preceding side, then it can't merge with us. + if let Some(ref inline_context) = other.inline_context { + for inline_context_node in inline_context.nodes.iter() { + if !inline_context_node.flags.contains(FIRST_FRAGMENT_OF_ELEMENT) { + continue + } + if inline_context_node.style.logical_margin().inline_start != + LengthOrPercentageOrAuto::Length(Au(0)) { + return false + } + if inline_context_node.style.logical_padding().inline_start != + LengthOrPercentage::Length(Au(0)) { + return false + } + if inline_context_node.style.logical_border_width().inline_start != Au(0) { + return false + } + } + } + + true } _ => false, } @@ -2286,6 +2377,29 @@ impl Fragment { pub fn is_absolutely_positioned(&self) -> bool { self.style.get_box().position == position::T::absolute } + + pub fn meld_with_next_inline_fragment(&mut self, next_fragment: &Fragment) { + if let Some(ref mut inline_context_of_this_fragment) = self.inline_context { + if let Some(ref inline_context_of_next_fragment) = next_fragment.inline_context { + for (i, inline_context_node_from_next_fragment) in + inline_context_of_next_fragment.nodes.iter().enumerate() { + if i >= inline_context_of_this_fragment.nodes.len() { + continue + } + if !inline_context_node_from_next_fragment.flags.contains( + LAST_FRAGMENT_OF_ELEMENT) { + continue + } + if inline_context_node_from_next_fragment.address != + inline_context_of_this_fragment.nodes[i].address { + continue + } + inline_context_of_this_fragment.nodes[i].flags.insert( + LAST_FRAGMENT_OF_ELEMENT); + } + } + } + } } impl fmt::Debug for Fragment { @@ -2378,7 +2492,7 @@ impl<'a> InlineStyleIterator<'a> { } } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum WhitespaceStrippingResult { RetainFragment, FragmentContainedOnlyBidiControlCharacters, diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 51739ede4c8..6066b4a8473 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1666,6 +1666,8 @@ impl Flow for InlineFlow { .early_absolute_position_info .relative_containing_block_mode, CoordinateSystem::Parent); + let stacking_relative_content_box = + fragment.stacking_relative_content_box(&stacking_relative_border_box); let clip = fragment.clipping_region_for_children(&self.base.clip, &stacking_relative_border_box, false); @@ -1689,7 +1691,7 @@ impl Flow for InlineFlow { } block_flow.base.stacking_relative_position = - stacking_relative_border_box.origin; + stacking_relative_content_box.origin; block_flow.base.stacking_relative_position_of_display_port = self.base.stacking_relative_position_of_display_port; } @@ -1817,6 +1819,20 @@ pub struct InlineFragmentNodeInfo { pub address: OpaqueNode, pub style: Arc, pub pseudo: PseudoElementType<()>, + pub flags: InlineFragmentNodeFlags, +} + +bitflags! { + flags InlineFragmentNodeFlags: u8 { + const FIRST_FRAGMENT_OF_ELEMENT = 0x01, + const LAST_FRAGMENT_OF_ELEMENT = 0x02, + } +} + +impl fmt::Debug for InlineFragmentNodeInfo { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:?}", self.flags.bits()) + } } #[derive(Clone)] diff --git a/components/layout/text.rs b/components/layout/text.rs index a61393b5d20..f0b64903b59 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -184,7 +184,6 @@ impl TextRunScanner { }; let (mut start_position, mut end_position) = (0, 0); - for character in text.chars() { // Search for the first font in this font group that contains a glyph for this // character. @@ -286,10 +285,15 @@ impl TextRunScanner { for (logical_offset, old_fragment) in mem::replace(&mut self.clump, LinkedList::new()).into_iter().enumerate() { loop { - match mappings.peek() { - Some(mapping) if mapping.old_fragment_index == logical_offset => {} - Some(_) | None => break, - }; + match mappings.peek() { + Some(mapping) if mapping.old_fragment_index == logical_offset => {} + Some(_) | None => { + if let Some(ref mut last_fragment) = out_fragments.last_mut() { + last_fragment.meld_with_next_inline_fragment(&old_fragment); + } + break; + } + }; let mut mapping = mappings.next().unwrap(); let run = runs[mapping.text_run_index].clone(); diff --git a/components/style/properties.mako.rs b/components/style/properties.mako.rs index 857718e7a44..5b305f490ec 100644 --- a/components/style/properties.mako.rs +++ b/components/style/properties.mako.rs @@ -6456,15 +6456,15 @@ pub fn modify_style_for_replaced_content(style: &mut Arc) { } } -/// Adjusts borders, padding, and margins as appropriate to account for a fragment's status as the -/// first or last fragment within the range of an element. +/// Adjusts borders as appropriate to account for a fragment's status as the first or last fragment +/// within the range of an element. /// -/// Specifically, this function sets border/padding/margin widths to zero on the sides for which -/// the fragment is not outermost. +/// Specifically, this function sets border widths to zero on the sides for which the fragment is +/// not outermost. #[inline] -pub fn modify_style_for_inline_sides(style: &mut Arc, - is_first_fragment_of_element: bool, - is_last_fragment_of_element: bool) { +pub fn modify_border_style_for_inline_sides(style: &mut Arc, + is_first_fragment_of_element: bool, + is_last_fragment_of_element: bool) { fn modify_side(style: &mut Arc, side: PhysicalSide) { let mut style = Arc::make_mut(style); let border = Arc::make_mut(&mut style.border); @@ -6472,34 +6472,18 @@ pub fn modify_style_for_inline_sides(style: &mut Arc, PhysicalSide::Left => { border.border_left_width = Au(0); border.border_left_style = BorderStyle::none; - Arc::make_mut(&mut style.padding).padding_left = - computed::LengthOrPercentage::Length(Au(0)); - Arc::make_mut(&mut style.margin).margin_left = - computed::LengthOrPercentageOrAuto::Length(Au(0)) } PhysicalSide::Right => { border.border_right_width = Au(0); border.border_right_style = BorderStyle::none; - Arc::make_mut(&mut style.padding).padding_right = - computed::LengthOrPercentage::Length(Au(0)); - Arc::make_mut(&mut style.margin).margin_right = - computed::LengthOrPercentageOrAuto::Length(Au(0)) } PhysicalSide::Bottom => { border.border_bottom_width = Au(0); border.border_bottom_style = BorderStyle::none; - Arc::make_mut(&mut style.padding).padding_bottom = - computed::LengthOrPercentage::Length(Au(0)); - Arc::make_mut(&mut style.margin).margin_bottom = - computed::LengthOrPercentageOrAuto::Length(Au(0)) } PhysicalSide::Top => { border.border_top_width = Au(0); border.border_top_style = BorderStyle::none; - Arc::make_mut(&mut style.padding).padding_top = - computed::LengthOrPercentage::Length(Au(0)); - Arc::make_mut(&mut style.margin).margin_top = - computed::LengthOrPercentageOrAuto::Length(Au(0)) } } } @@ -6534,7 +6518,7 @@ pub fn modify_style_for_outer_inline_block_fragment(style: &mut Arc) { position_offsets.bottom = computed::LengthOrPercentageOrAuto::Auto; position_offsets.left = computed::LengthOrPercentageOrAuto::Auto; } + + if style.padding.padding_top != computed::LengthOrPercentage::Length(Au(0)) || + style.padding.padding_right != computed::LengthOrPercentage::Length(Au(0)) || + style.padding.padding_bottom != computed::LengthOrPercentage::Length(Au(0)) || + style.padding.padding_left != computed::LengthOrPercentage::Length(Au(0)) { + let mut style = Arc::make_unique(style); + let mut padding = Arc::make_unique(&mut style.padding); + padding.padding_top = computed::LengthOrPercentage::Length(Au(0)); + padding.padding_right = computed::LengthOrPercentage::Length(Au(0)); + padding.padding_bottom = computed::LengthOrPercentage::Length(Au(0)); + padding.padding_left = computed::LengthOrPercentage::Length(Au(0)); + } } /// Adjusts the `margin` property as necessary to account for the text of an `input` element. diff --git a/tests/ref/basic.list b/tests/ref/basic.list index c7aeb3a1de8..e68864215a6 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -224,6 +224,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html != list_style_type_a.html list_style_type_ref.html == many_brs_a.html many_brs_ref.html == margin_a.html margin_b.html +== margin_padding_inline_block_a.html margin_padding_inline_block_ref.html == margins_inside_floats_a.html margins_inside_floats_ref.html == marker_block_direction_placement_a.html marker_block_direction_placement_ref.html == max_width_float_simple_a.html max_width_float_simple_b.html diff --git a/tests/ref/input_button_margins_a.html b/tests/ref/input_button_margins_a.html index b35a3b5ccb6..a207daac455 100644 --- a/tests/ref/input_button_margins_a.html +++ b/tests/ref/input_button_margins_a.html @@ -5,6 +5,8 @@ body, html { } input { margin-left: 64px; + border: none; + vertical-align: top; } diff --git a/tests/ref/input_button_margins_ref.html b/tests/ref/input_button_margins_ref.html index 07ecda7c227..63b80497bf4 100644 --- a/tests/ref/input_button_margins_ref.html +++ b/tests/ref/input_button_margins_ref.html @@ -6,6 +6,7 @@ body, html { input { position: absolute; left: 64px; + border: none; } diff --git a/tests/ref/margin_padding_inline_block_a.html b/tests/ref/margin_padding_inline_block_a.html new file mode 100644 index 00000000000..d2a8f5d4b82 --- /dev/null +++ b/tests/ref/margin_padding_inline_block_a.html @@ -0,0 +1,10 @@ + + +
Boo
+
Foo
+
Foo
+ diff --git a/tests/ref/margin_padding_inline_block_ref.html b/tests/ref/margin_padding_inline_block_ref.html new file mode 100644 index 00000000000..58f5ba65980 --- /dev/null +++ b/tests/ref/margin_padding_inline_block_ref.html @@ -0,0 +1,17 @@ + + +
Boo
+
Foo
+
Foo
+