Commit 947ca034 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[LayoutNG] Fix coordinate space of NG LocalCaretRect() in vertical-rl

LocalCaretRect should use the "flipped blocks" coordinate space for
vertical-rl writing mode. This patch applies that in the NG implementation.

Note: this patch only changes behavior for text-anchored carets. When
caret is anchored to atomic inline, there is no reference as the legacy
behavior is broken, and LocalCaretRect is consumed by legacy.

Bug: 822575
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I4019f560f8410ef14c7d52312d7fc1de8db6ee28
Reviewed-on: https://chromium-review.googlesource.com/1011296Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551528}
parent a6ae3a1c
...@@ -201,8 +201,7 @@ TEST_P(ParameterizedLocalCaretRectTest, UnderflowTextRtl) { ...@@ -201,8 +201,7 @@ TEST_P(ParameterizedLocalCaretRectTest, UnderflowTextRtl) {
Position(text, 2), TextAffinity::kDownstream))); Position(text, 2), TextAffinity::kDownstream)));
} }
// TODO(xiaochengh): Fix NG LocalCaretText computation for vertical text. TEST_P(ParameterizedLocalCaretRectTest, VerticalRLText) {
TEST_F(LocalCaretRectTest, VerticalRLText) {
// This test only records the current behavior. Future changes are allowed. // This test only records the current behavior. Future changes are allowed.
LoadAhem(); LoadAhem();
...@@ -211,11 +210,6 @@ TEST_F(LocalCaretRectTest, VerticalRLText) { ...@@ -211,11 +210,6 @@ TEST_F(LocalCaretRectTest, VerticalRLText) {
"font: 10px/10px Ahem; width: 30px; height: 30px'>XXXYYYZZZ</div>"); "font: 10px/10px Ahem; width: 30px; height: 30px'>XXXYYYZZZ</div>");
const Node* foo = GetElementById("div")->firstChild(); const Node* foo = GetElementById("div")->firstChild();
// TODO(xiaochengh): In vertical-rl writing mode, the |X| property of
// LocalCaretRect's LayoutRect seems to refer to the distance to the right
// edge of the inline formatting context instead. Figure out if this is
// correct.
EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(), LayoutRect(0, 0, 10, 1)), EXPECT_EQ(LocalCaretRect(foo->GetLayoutObject(), LayoutRect(0, 0, 10, 1)),
LocalCaretRectOfPosition(PositionWithAffinity( LocalCaretRectOfPosition(PositionWithAffinity(
Position(foo, 0), TextAffinity::kDownstream))); Position(foo, 0), TextAffinity::kDownstream)));
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
#include "third_party/blink/renderer/core/editing/local_caret_rect.h" #include "third_party/blink/renderer/core/editing/local_caret_rect.h"
#include "third_party/blink/renderer/core/editing/position_with_affinity.h" #include "third_party/blink/renderer/core/editing/position_with_affinity.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h" #include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/layout/layout_object.h" #include "third_party/blink/renderer/core/layout/layout_block_flow.h"
#include "third_party/blink/renderer/core/layout/ng/geometry/ng_physical_offset_rect.h" #include "third_party/blink/renderer/core/layout/ng/geometry/ng_physical_offset_rect.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_caret_position.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_caret_position.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h"
...@@ -120,29 +120,38 @@ LocalCaretRect ComputeLocalCaretRect(const NGCaretPosition& caret_position) { ...@@ -120,29 +120,38 @@ LocalCaretRect ComputeLocalCaretRect(const NGCaretPosition& caret_position) {
if (caret_position.IsNull()) if (caret_position.IsNull())
return LocalCaretRect(); return LocalCaretRect();
const NGPaintFragment& fragment = *caret_position.fragment;
const LayoutObject* layout_object = fragment.GetLayoutObject();
switch (caret_position.position_type) { switch (caret_position.position_type) {
case NGCaretPositionType::kBeforeBox: case NGCaretPositionType::kBeforeBox:
case NGCaretPositionType::kAfterBox: { case NGCaretPositionType::kAfterBox: {
DCHECK(caret_position.fragment->PhysicalFragment().IsBox()); DCHECK(fragment.PhysicalFragment().IsBox());
const NGPhysicalOffsetRect fragment_local_rect = const NGPhysicalOffsetRect fragment_local_rect =
ComputeLocalCaretRectByBoxSide(*caret_position.fragment, ComputeLocalCaretRectByBoxSide(fragment,
caret_position.position_type); caret_position.position_type);
return {caret_position.fragment->GetLayoutObject(), return {layout_object, fragment_local_rect.ToLayoutRect()};
fragment_local_rect.ToLayoutRect()};
} }
case NGCaretPositionType::kAtTextOffset: { case NGCaretPositionType::kAtTextOffset: {
DCHECK(caret_position.fragment->PhysicalFragment().IsText()); DCHECK(fragment.PhysicalFragment().IsText());
DCHECK(caret_position.text_offset.has_value()); DCHECK(caret_position.text_offset.has_value());
const NGPhysicalOffsetRect caret_rect = ComputeLocalCaretRectAtTextOffset( const NGPhysicalOffsetRect caret_rect = ComputeLocalCaretRectAtTextOffset(
*caret_position.fragment, *caret_position.text_offset); fragment, *caret_position.text_offset);
LayoutRect layout_rect = caret_rect.ToLayoutRect();
return {caret_position.fragment->GetLayoutObject(),
caret_rect.ToLayoutRect()}; // For vertical-rl, convert to "flipped block-flow" coordinates space.
// See core/layout/README.md#coordinate-spaces for details.
if (fragment.Style().IsFlippedBlocksWritingMode()) {
const LayoutBlockFlow* container =
layout_object->EnclosingNGBlockFlow();
container->FlipForWritingMode(layout_rect);
}
return {layout_object, layout_rect};
} }
} }
NOTREACHED(); NOTREACHED();
return {caret_position.fragment->GetLayoutObject(), LayoutRect()}; return {layout_object, LayoutRect()};
} }
} // namespace } // namespace
......
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