Skip to content

Commit

Permalink
<ruby>: Introduce |text-align: -internal-space-around|
Browse files Browse the repository at this point in the history
This CL adds |-internal-space-around| keyword to |text-align| CSS
property. It is used for the default value of ruby annotation boxes
and ruby base boxes.

We assumed |start|, the initial value of text-align, in ComputedStyle
had a special effect for ruby annotation and ruby base, and we made a
variant of kJustify internally.  The code was complicated, and
introducing |-internal-space-around| simplifies the code.

This CL has a small behavior change:
 - If text-align:start is specified to <rt>, we ignored it before this
   CL and applied the 'space-around' behavior.
   After this CL, we apply text-align:start behavior in that case.

* LayoutRubyRun.cc
  ComputedStyle for LayoutRubyBase was sometimes overwritten by
  LayoutObject::PropagateStyleToAnonymousChildren(), and the TextAlign
  value was reset to kStart. Uses UpdateAnonymousChildStyle() to keep
  the expected TextAlign value.

* insert-paragraph-into-table-expected.txt
  |style="text-align: -internal-space-around;"|, which is unable to
  parse, appears.  This will be fixed when we implement ruby-align CSS
  property.

Bug: 1069817
Change-Id: Iecc1e999d11b58dd4d950f3579310546f5db7585
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2217673
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773079}
  • Loading branch information
tkent-google authored and Commit Bot committed May 29, 2020
1 parent 93e63fb commit 17ef075
Show file tree
Hide file tree
Showing 24 changed files with 89 additions and 150 deletions.
5 changes: 3 additions & 2 deletions third_party/blink/renderer/core/css/css_properties.json5
Original file line number Diff line number Diff line change
Expand Up @@ -3518,7 +3518,7 @@
field_template: "keyword",
keywords: [
"left", "right", "center", "justify", "-webkit-left", "-webkit-right",
"-webkit-center", "start", "end"
"-webkit-center", "start", "end", "-internal-space-around"
],
typedom_types: ["Keyword"],
default_value: "start",
Expand All @@ -3531,7 +3531,8 @@
inherited: true,
field_group: "*",
field_template: "keyword",
keywords: ["auto", "start", "end", "left", "right", "center", "justify"],
keywords: ["auto", "start", "end", "left", "right", "center", "justify",
"-internal-space-around"],
default_value: "auto",
typedom_types: ["Keyword"]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@
"-webkit-center",
"-webkit-match-parent",
"-internal-center",
"-internal-space-around",
//
// text-justify
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,13 +735,14 @@ bool CSSParserFastPaths::IsValidKeywordPropertyAndValue(
return value_id == CSSValueID::kAuto || value_id == CSSValueID::kFixed;
case CSSPropertyID::kTextAlign:
return (value_id >= CSSValueID::kWebkitAuto &&
value_id <= CSSValueID::kInternalCenter) ||
value_id <= CSSValueID::kInternalSpaceAround) ||
value_id == CSSValueID::kStart || value_id == CSSValueID::kEnd;
case CSSPropertyID::kTextAlignLast:
return (value_id >= CSSValueID::kLeft &&
value_id <= CSSValueID::kJustify) ||
value_id == CSSValueID::kStart || value_id == CSSValueID::kEnd ||
value_id == CSSValueID::kAuto;
value_id == CSSValueID::kAuto ||
value_id == CSSValueID::kInternalSpaceAround;
case CSSPropertyID::kTextAnchor:
return value_id == CSSValueID::kStart ||
value_id == CSSValueID::kMiddle || value_id == CSSValueID::kEnd;
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/editing/editing_style.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,8 @@ static CSSValueID TextAlignResolvingStartAndEnd(CSSValueID text_align,
return CSSValueID::kCenter;
case CSSValueID::kJustify:
return CSSValueID::kJustify;
case CSSValueID::kInternalSpaceAround:
return CSSValueID::kInternalSpaceAround;
case CSSValueID::kLeft:
case CSSValueID::kWebkitLeft:
return CSSValueID::kLeft;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/resources/html.css
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ rt {
ruby > rt {
display: block;
font-size: 50%;
text-align: start;
text-align: -internal-space-around;
}

rp {
Expand Down
5 changes: 0 additions & 5 deletions third_party/blink/renderer/core/layout/layout_block.h
Original file line number Diff line number Diff line change
Expand Up @@ -402,11 +402,6 @@ class CORE_EXPORT LayoutBlock : public LayoutBox {
MinMaxSizes PreferredLogicalWidths() const override;

protected:
virtual void AdjustInlineDirectionLineBounds(
unsigned /* expansionOpportunityCount */,
LayoutUnit& /* logicalLeft */,
LayoutUnit& /* logicalWidth */) const {}

MinMaxSizes ComputeIntrinsicLogicalWidths() const override;
void ComputeChildPreferredLogicalWidths(
LayoutObject& child,
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/core/layout/layout_block_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,6 @@ class CORE_EXPORT LayoutBlockFlow : public LayoutBlock {
}
}

virtual ETextAlign TextAlignmentForLine(bool ends_with_soft_break) const;

private:
LayoutUnit CollapsedMarginBefore() const final {
return MaxPositiveMarginBefore() - MaxNegativeMarginBefore();
Expand Down
42 changes: 30 additions & 12 deletions third_party/blink/renderer/core/layout/layout_block_flow_line.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,6 @@ RootInlineBox* LayoutBlockFlow::ConstructLine(BidiRunList<BidiRun>& bidi_runs,
return LastRootBox();
}

ETextAlign LayoutBlockFlow::TextAlignmentForLine(
bool ends_with_soft_break) const {
return StyleRef().GetTextAlign(!ends_with_soft_break);
}

static bool TextAlignmentNeedsTrailingSpace(ETextAlign text_align,
const ComputedStyle& style) {
switch (text_align) {
Expand All @@ -379,6 +374,7 @@ static bool TextAlignmentNeedsTrailingSpace(ETextAlign text_align,
case ETextAlign::kCenter:
case ETextAlign::kWebkitCenter:
case ETextAlign::kJustify:
case ETextAlign::kInternalSpaceAround:
return style.CollapseWhiteSpace();
case ETextAlign::kStart:
return style.CollapseWhiteSpace() && !style.IsLeftToRightDirection();
Expand Down Expand Up @@ -691,9 +687,30 @@ void LayoutBlockFlow::UpdateLogicalWidthForAlignment(
StyleRef().IsLeftToRightDirection(), trailing_space_run, logical_left,
total_logical_width, available_logical_width);
break;

case ETextAlign::kInternalSpaceAround: {
int max_preferred_logical_width =
PreferredLogicalWidths().max_size.ToInt();
if (max_preferred_logical_width < available_logical_width) {
unsigned capped_count =
static_cast<unsigned>(LayoutUnit::Max().Floor());
capped_count = std::min(expansion_opportunity_count, capped_count);

// Inset the line by half the inter-ideograph expansion amount.
LayoutUnit inset =
(available_logical_width - max_preferred_logical_width) /
(capped_count + 1);
// For <rt>, inset it by no more than a full-width ruby character on
// each side.
if (IsRubyText() && capped_count > 0)
inset = std::min(LayoutUnit(2 * StyleRef().FontSize()), inset);

logical_left += inset / 2;
available_logical_width -= inset;
}
FALLTHROUGH;
}
case ETextAlign::kJustify:
AdjustInlineDirectionLineBounds(expansion_opportunity_count, logical_left,
available_logical_width);
if (expansion_opportunity_count) {
if (trailing_space_run) {
total_logical_width -= trailing_space_run->box_->LogicalWidth();
Expand Down Expand Up @@ -821,8 +838,9 @@ BidiRun* LayoutBlockFlow::ComputeInlineDirectionPositionsForSegment(
}
if (r->line_layout_item_.IsText()) {
LineLayoutText rt(r->line_layout_item_);
if (text_align == ETextAlign::kJustify && r != trailing_space_run &&
text_justify != TextJustify::kNone) {
if ((text_align == ETextAlign::kJustify ||
text_align == ETextAlign::kInternalSpaceAround) &&
r != trailing_space_run && text_justify != TextJustify::kNone) {
if (!is_after_expansion)
ToInlineTextBox(r->box_)->SetCanHaveLeadingExpansion(true);
expansions.AddRunWithExpansions(*r, is_after_expansion, text_justify);
Expand Down Expand Up @@ -1166,9 +1184,9 @@ void LayoutBlockFlow::LayoutRunsAndFloatsInRange(
BidiStatus(direction, IsOverride(style_to_use.GetUnicodeBidi())));
}

ETextAlign text_align = TextAlignmentForLine(
!end_of_line.AtEnd() &&
!layout_state.GetLineInfo().PreviousLineBrokeCleanly());
ETextAlign text_align = StyleRef().GetTextAlign(
end_of_line.AtEnd() ||
layout_state.GetLineInfo().PreviousLineBrokeCleanly());
layout_state.GetLineInfo().SetTextAlign(text_align);
resolver.SetNeedsTrailingSpace(
TextAlignmentNeedsTrailingSpace(text_align, style_to_use));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,7 @@ LayoutRect LayoutBoxModelObject::LocalCaretRectForEmptyElement(
alignment = kAlignRight;
break;
case ETextAlign::kJustify:
case ETextAlign::kInternalSpaceAround:
case ETextAlign::kStart:
if (!current_style.IsLeftToRightDirection())
alignment = kAlignRight;
Expand Down
25 changes: 0 additions & 25 deletions third_party/blink/renderer/core/layout/layout_ruby_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,29 +154,4 @@ void LayoutRubyBase::MoveBlockChildren(LayoutRubyBase* to_base,
}
}

ETextAlign LayoutRubyBase::TextAlignmentForLine(
bool /* endsWithSoftBreak */) const {
return ETextAlign::kJustify;
}

void LayoutRubyBase::AdjustInlineDirectionLineBounds(
unsigned expansion_opportunity_count,
LayoutUnit& logical_left,
LayoutUnit& logical_width) const {
int max_preferred_logical_width = PreferredLogicalWidths().max_size.ToInt();
if (max_preferred_logical_width >= logical_width)
return;

unsigned max_count = static_cast<unsigned>(LayoutUnit::Max().Floor());
if (expansion_opportunity_count > max_count)
expansion_opportunity_count = max_count;

// Inset the ruby base by half the inter-ideograph expansion amount.
LayoutUnit inset = (logical_width - max_preferred_logical_width) /
(expansion_opportunity_count + 1);

logical_left += inset / 2;
logical_width -= inset;
}

} // namespace blink
6 changes: 0 additions & 6 deletions third_party/blink/renderer/core/layout/layout_ruby_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ class LayoutRubyBase : public LayoutBlockFlow {
// constructor.
explicit LayoutRubyBase(Element*);

ETextAlign TextAlignmentForLine(bool ends_with_soft_break) const override;
void AdjustInlineDirectionLineBounds(
unsigned expansion_opportunity_count,
LayoutUnit& logical_left,
LayoutUnit& logical_width) const override;

void MoveChildren(LayoutRubyBase* to_base,
LayoutObject* before_child = nullptr);
void MoveInlineChildren(LayoutRubyBase* to_base,
Expand Down
10 changes: 9 additions & 1 deletion third_party/blink/renderer/core/layout/layout_ruby_run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,19 @@ LayoutRubyBase* LayoutRubyRun::CreateRubyBase() const {
scoped_refptr<ComputedStyle> new_style =
ComputedStyle::CreateAnonymousStyleWithDisplay(StyleRef(),
EDisplay::kBlock);
new_style->SetTextAlign(ETextAlign::kCenter); // FIXME: use WEBKIT_CENTER?
UpdateAnonymousChildStyle(layout_object, *new_style);
layout_object->SetStyle(std::move(new_style));
return layout_object;
}

void LayoutRubyRun::UpdateAnonymousChildStyle(
const LayoutObject* child,
ComputedStyle& child_style) const {
DCHECK(child->IsRubyBase()) << child;
child_style.SetDisplay(EDisplay::kBlock);
child_style.SetTextAlign(ETextAlign::kInternalSpaceAround);
}

LayoutRubyRun* LayoutRubyRun::StaticCreateRubyRun(
const LayoutObject* parent_ruby,
const LayoutBlock& containing_block) {
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/layout/layout_ruby_run.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class LayoutRubyRun : public LayoutBlockFlow {
}
bool CreatesAnonymousWrapper() const override { return true; }
void RemoveLeftoverAnonymousBlock(LayoutBlock*) override {}
void UpdateAnonymousChildStyle(const LayoutObject* child,
ComputedStyle& child_style) const override;

friend class LayoutNGMixin<LayoutRubyRun>;
};
Expand Down
37 changes: 0 additions & 37 deletions third_party/blink/renderer/core/layout/layout_ruby_text.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,41 +42,4 @@ bool LayoutRubyText::IsChildAllowed(LayoutObject* child,
return child->IsInline();
}

ETextAlign LayoutRubyText::TextAlignmentForLine(
bool ends_with_soft_break) const {
ETextAlign text_align = StyleRef().GetTextAlign();
// FIXME: This check is bogus since user can set the initial value.
if (text_align != ComputedStyleInitialValues::InitialTextAlign())
return LayoutBlockFlow::TextAlignmentForLine(ends_with_soft_break);

// The default behavior is to allow ruby text to expand if it is shorter than
// the ruby base.
return ETextAlign::kJustify;
}

void LayoutRubyText::AdjustInlineDirectionLineBounds(
unsigned expansion_opportunity_count,
LayoutUnit& logical_left,
LayoutUnit& logical_width) const {
ETextAlign text_align = StyleRef().GetTextAlign();
// FIXME: This check is bogus since user can set the initial value.
if (text_align != ComputedStyleInitialValues::InitialTextAlign())
return LayoutBlockFlow::AdjustInlineDirectionLineBounds(
expansion_opportunity_count, logical_left, logical_width);

int max_preferred_logical_width = PreferredLogicalWidths().max_size.ToInt();
if (max_preferred_logical_width >= logical_width)
return;

// Inset the ruby text by half the inter-ideograph expansion amount, but no
// more than a full-width ruby character on each side.
LayoutUnit inset = (logical_width - max_preferred_logical_width) /
(expansion_opportunity_count + 1);
if (expansion_opportunity_count)
inset = std::min(LayoutUnit(2 * StyleRef().FontSize()), inset);

logical_left += inset / 2;
logical_width -= inset;
}

} // namespace blink
7 changes: 0 additions & 7 deletions third_party/blink/renderer/core/layout/layout_ruby_text.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,6 @@ class LayoutRubyText : public LayoutBlockFlow {
// ruby text objects.
return true;
}

private:
ETextAlign TextAlignmentForLine(bool ends_with_soft_break) const override;
void AdjustInlineDirectionLineBounds(
unsigned expansion_opportunity_count,
LayoutUnit& logical_left,
LayoutUnit& logical_width) const override;
};

DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutRubyText, IsRubyText());
Expand Down
5 changes: 4 additions & 1 deletion third_party/blink/renderer/core/layout/layout_text.cc
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,7 @@ LayoutRect LayoutText::LocalCaretRect(
case ETextAlign::kWebkitCenter:
break;
case ETextAlign::kJustify:
case ETextAlign::kInternalSpaceAround:
case ETextAlign::kStart:
right_aligned = !cb_style.IsLeftToRightDirection();
break;
Expand Down Expand Up @@ -1673,7 +1674,8 @@ bool LayoutText::CanOptimizeSetText() const {
// We would need to recompute the position if "direction" is "rtl".
StyleRef().IsLeftToRightDirection() &&
// We would need to layout the text if it is justified.
(StyleRef().GetTextAlign(true) != ETextAlign::kJustify);
(StyleRef().GetTextAlign(true) != ETextAlign::kJustify &&
StyleRef().GetTextAlign(true) != ETextAlign::kInternalSpaceAround);
}

void LayoutText::SetFirstTextBoxLogicalLeft(float text_width) const {
Expand All @@ -1691,6 +1693,7 @@ void LayoutText::SetFirstTextBoxLogicalLeft(float text_width) const {
case ETextAlign::kLeft:
case ETextAlign::kWebkitLeft:
case ETextAlign::kJustify:
case ETextAlign::kInternalSpaceAround:
case ETextAlign::kStart:
// Do nothing.
break;
Expand Down
Loading

0 comments on commit 17ef075

Please sign in to comment.