Skip to content

GitLab

T
tangled
Commit 4b6ad35e authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot
Browse files
Options

Use paragraph base direction always for trailing spaces runs. 

As per Unicode UAX#9 L1.4, line trailing whitespaces should always have
the direction of the paragraph no matter the direction they had before
line breaking.

To accomplish that, in inline layout algorithm CreateLine, when an
NGInlineItemResult is only trailing spaces, we ignore the bidi
level coming from the NGInlineItem, and just use the base direction
of the line.

This should fix the case of an LTR line with trailing space and
pre-wrap in an RTL line that would end up putting the trailing
space to the right of the last LTR line instead of to the left of
the line.

This fixes css/css-text/white-space/eol-spaces-bidi-002.html with
width of 7 characters. It is still not fixing the case for 8
characters.

Bug: 316409, 1142926
Change-Id: Iab220f79ba0b2526fd51d5341363cc8e9087e133
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505486
Commit-Queue: José Dapena Paz <jdapena@igalia.com>
Reviewed-by: default avatarJavier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824804}
parent 68c03e70
Hide whitespace changes
Inline Side-by-side
Showing with 104 additions and 7 deletions
third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc
View file @ 4b6ad35e
......@@ -243,16 +243,15 @@ void NGInlineLayoutAlgorithm::CreateLine(
DCHECK(item_result.hyphen_string);
DCHECK(item_result.hyphen_shape_result);
LayoutUnit hyphen_inline_size = item_result.HyphenInlineSize();
line_box->AddChild(item, std::move(item_result.shape_result),
item_result.TextOffset(), box->text_top,
line_box->AddChild(item, item_result, item_result.TextOffset(),
box->text_top,
item_result.inline_size - hyphen_inline_size,
box->text_height, item.BidiLevel());
PlaceHyphen(item_result, hyphen_inline_size, line_box, box);
} else {
line_box->AddChild(item, std::move(item_result.shape_result),
item_result.TextOffset(), box->text_top,
item_result.inline_size, box->text_height,
item.BidiLevel());
line_box->AddChild(item, item_result, item_result.TextOffset(),
box->text_top, item_result.inline_size,
box->text_height, item.BidiLevel());
}
has_logical_text_items = true;
......@@ -1248,6 +1247,11 @@ void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction,
constexpr UBiDiLevel kOpaqueBidiLevel = 0xff;
DCHECK_GT(kOpaqueBidiLevel, UBIDI_MAX_EXPLICIT_LEVEL + 1);
// The base direction level is used for the items that should ignore its
// original level and just use the paragraph level, as trailing opaque
// items and items with only trailing whitespaces.
UBiDiLevel base_direction_level = IsLtr(base_direction) ? 0 : 1;
// Create a list of chunk indices in the visual order.
// ICU |ubidi_getVisualMap()| works for a run of characters. Since we can
// handle the direction of each run, we use |ubidi_reorderVisual()| to reorder
......@@ -1262,13 +1266,18 @@ void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction,
continue;
}
DCHECK_NE(item.bidi_level, kOpaqueBidiLevel);
// UAX#9 L1: trailing whitespaces should use paragraph direction.
if (item.has_only_trailing_spaces) {
levels.push_back(base_direction_level);
continue;
}
levels.push_back(item.bidi_level);
}
// For opaque items, copy bidi levels from adjacent items.
if (has_opaque_items) {
// Use the paragraph level for trailing opaque items.
UBiDiLevel last_level = IsLtr(base_direction) ? 0 : 1;
UBiDiLevel last_level = base_direction_level;
for (UBiDiLevel& level : base::Reversed(levels)) {
if (level == kOpaqueBidiLevel)
level = last_level;
......
third_party/blink/renderer/core/layout/ng/inline/ng_logical_line_item.h
View file @ 4b6ad35e
......@@ -7,6 +7,7 @@
#include "third_party/blink/renderer/core/layout/geometry/logical_rect.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_item.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_item_result.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h"
#include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
......@@ -49,6 +50,20 @@ struct NGLogicalLineItem {
children_count(children_count),
bidi_level(bidi_level) {}
// Create an in-flow text fragment.
NGLogicalLineItem(const NGInlineItem& inline_item,
NGInlineItemResult& item_result,
const NGTextOffset& text_offset,
LayoutUnit block_offset,
LayoutUnit inline_size,
LayoutUnit text_height,
UBiDiLevel bidi_level)
: inline_item(&inline_item),
shape_result(std::move(item_result.shape_result)),
text_offset(text_offset),
rect(LayoutUnit(), block_offset, LayoutUnit(), text_height),
inline_size(inline_size),
bidi_level(bidi_level),
has_only_trailing_spaces(item_result.has_only_trailing_spaces) {}
NGLogicalLineItem(const NGInlineItem& inline_item,
scoped_refptr<const ShapeResultView> shape_result,
const NGTextOffset& text_offset,
......@@ -215,6 +230,9 @@ struct NGLogicalLineItem {
UBiDiLevel bidi_level = 0xff;
// The current text direction for OOF positioned items.
TextDirection container_direction = TextDirection::kLtr;
// When an item contains only trailing spaces, the original bidi level needs
// to be ignored, and just use paragraph direction (UAX#9 L1)
bool has_only_trailing_spaces = false;
bool is_hidden_for_paint = false;
};
......
third_party/blink/web_tests/external/wpt/css/css-text/white-space/eol-spaces-bidi-003.html 0 → 100644
View file @ 4b6ad35e
<!doctype html>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>CSS Text 3 test: trailing collapsible spaces and bidi</title>
<link rel="author" title="Jose Dapena Paz" href="mailto:jdapena@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#valdef-white-space-pre-wrap">
<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-phase-2">
<link rel="match" href="reference/eol-spaces-bidi-003-ref.html">
<link rel="match" href="reference/eol-spaces-bidi-alt-003-ref.html">
<meta name="assert" content="Hanging space between OP and D should take paragraph direction and show in the end of the line (blue box to the left)">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
div {
direction: rtl;
font: 20px/1 Ahem;
margin-left: 20px;
background: green;
width: 4ch;
}
.blue { color: blue; }
.bgblue { background-color: blue; }
.space { color: transparent; }
</style>
<p>Test passes if a blue box (the white space) is visible at the left start in first line.</p>
<div>ب<span class="bgblue"><span class="blue">X</span>OP</span><span class="space">X<br>XXX</span>D</div>
third_party/blink/web_tests/external/wpt/css/css-text/white-space/reference/eol-spaces-bidi-003-ref.html 0 → 100644
View file @ 4b6ad35e
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text 3 test reference</title>
<link rel="author" title="Jose Dapena Paz" href="mailto:jdapena@igalia.com">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
div {
direction: rtl;
font: 20px/1 Ahem;
margin-left: 20px;
background: green;
width: 4ch;
}
.blue { color: blue; }
.space { color: transparent; }
</style>
<p>Test passes if a blue box (the white space) is visible at the left start in first line.</p>
<div>ب<span class="blue">X</span>OP<span class="space">X<br>XXX</span>D</div>
third_party/blink/web_tests/external/wpt/css/css-text/white-space/reference/eol-spaces-bidi-alt-003-ref.html 0 → 100644
View file @ 4b6ad35e
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Text 3 test reference</title>
<link rel="author" title="Jose Dapena Paz" href="mailto:jdapena@igalia.com">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
<style>
div {
direction: rtl;
font: 20px/1 Ahem;
margin-left: 20px;
background: green;
width: 4ch;
overflow: hidden;
}
.blue { color: blue; }
.space { color: transparent; }
</style>
<p>Test passes if a blue box (the white space) is visible at the left start in first line.</p>
<div>ب<span class="blue">X</span>OP<span class="space">X<br>XXX</span>D</div>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment