auto merge of #1349 : dhedlund/servo/issue_222, r=jdm,me

Fixes #222.

Implements 'text-align' inheritance for inline elements.  The text alignment code has been changed a couple of times since the ticket was originally opened and was improved to inherit the text-align property from from an ancestor element.  This addressed the issue, so long as no inline elements had a text-align property defined.  Both Firefox and Chromium ignore any text-align properties attached to inline elements, but none of the specs are explicit about ignoring the property or inheriting only from non-inline elements:
http://www.w3.org/TR/CSS2/visuren.html#inline-formatting
http://www.w3.org/TR/CSS2/text.html#propdef-text-align

This is my first stab at writing any code that touches the DOM node tree.  Based on a few observations, I made the assumption that the `self.base.node` of a InlineFlow always corresponds to a BlockFlow-based node, no matter how deep inline DOM elements are nested; there was no obvious way to traverse Flow-traited objects directly and I'm not sure if it's possible to get from a FlowData struct back to a non-child {Block,Inline}Flow (probably intentionally).

I could've kept traversing the node tree directly, checking against each node style to ensure it didn't have an inline display property, but I could not create a scenario where the `base.node` was ever an inline.  It also feels like a code smell to be walking up the tree at all, especially for stylesheet properties.  Feels like it should eventually be handled in style::properties directly as being conditional inherited?
This commit is contained in:
bors-servo 2013-12-13 09:07:11 -08:00
commit c67669fe73
4 changed files with 46 additions and 14 deletions

View file

@ -579,22 +579,10 @@ impl InlineFlow {
}
/// Sets box X positions based on alignment for one line.
fn set_horizontal_box_positions(boxes: &[Box], line: &LineBox) {
fn set_horizontal_box_positions(boxes: &[Box], line: &LineBox, linebox_align: text_align::T) {
// Figure out how much width we have.
let slack_width = Au::max(Au(0), line.green_zone.width - line.bounds.size.width);
// Get the text alignment.
//
// TODO(burg, issue #222): use 'text-align' property from `InlineFlow`'s block container,
// not from the style of the first box child.
let linebox_align = if line.range.begin() < boxes.len() {
let first_box = &boxes[line.range.begin()];
first_box.style().Text.text_align
} else {
// Nothing to lay out, so assume left alignment.
text_align::left
};
// Set the box x positions based on that alignment.
let mut offset_x = line.bounds.origin.x;
offset_x = offset_x + match linebox_align {
@ -712,10 +700,13 @@ impl Flow for InlineFlow {
let mut line_height_offset = Au::new(0);
// All lines use text alignment from base (non-inline) node
let text_align = self.base.node.style().get().Text.text_align;
// Now, go through each line and lay out the boxes inside.
for line in self.lines.mut_iter() {
// Lay out boxes horizontally.
InlineFlow::set_horizontal_box_positions(self.boxes, line);
InlineFlow::set_horizontal_box_positions(self.boxes, line, text_align);
// Set the top y position of the current linebox.
// `line_height_offset` is updated at the end of the previous loop.

View file

@ -18,3 +18,4 @@
== border_style_none_a.html border_style_none_b.html
== acid1_a.html acid1_b.html
== text_decoration_propagation_a.html text_decoration_propagation_b.html
== inline_text_align_a.html inline_text_align_b.html

View file

@ -0,0 +1,24 @@
<html>
<head>
<style>
.right {
text-align: right;
}
.left {
text-align: left;
}
.inline {
display: inline;
}
</style>
</head>
<body>
<div class="right">
<span class="left"> <!-- should ignore 'text-align' of inline elements -->
<div class="inline left"> <!-- ... and those explicitly defined as inline -->
<span>hello, wide world</span>
</span>
</span>
</div>
</body>
</html>

View file

@ -0,0 +1,16 @@
<html>
<head>
<style>
.right {
text-align: right;
}
</style>
</head>
<body>
<div class="right">
<span class="right">
hello, wide world
</span>
</div>
</body>
</html>