Commit 1d2dccbc authored by yoichio's avatar yoichio Committed by Commit Bot

Range::BoundingRect() must independent from Selection.

Range::BoundingRect() returns different Rect depending on whether 
included Text nodes are selected or not. 
Since Range itself lives independently from user selection,
 this function also should be away from Selection.

The root cause is InlineTextBox::LocalSelectionRect(), which is
 called by BoundingRect(), depends on SelectionState.
LocalSelectionRect() refers current SelectionState so that
 paint invalidation invalids wrapping  line endings.

This patch change LocalSelectionRect(start,end) to have another
 boolean option which decides if we use current SelectionState.
(core/layout/*)

This patch also fixes InlineTextBox::GetSelectionState() can 
 update SelectionState by calling LayoutSelection::SelectionStartEnd().
Callers of SelectionStartEnd() must know SelectionState but 
 Commit() in SelectionStartEnd() can change the state.
(core/editing/LayoutSelection.*)


Bug: 739062
Change-Id: I98987940cd014d302c13d1370111a6664740e646
Reviewed-on: https://chromium-review.googlesource.com/567791
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486277}
parent f75b7670
......@@ -5,11 +5,15 @@
#include "core/dom/Range.h"
#include "bindings/core/v8/ExceptionState.h"
#include "bindings/core/v8/StringOrArrayBufferOrArrayBufferView.h"
#include "bindings/core/v8/V8BindingForTesting.h"
#include "core/css/FontFaceDescriptors.h"
#include "core/css/FontFaceSet.h"
#include "core/dom/Element.h"
#include "core/dom/NodeList.h"
#include "core/dom/Text.h"
#include "core/editing/EditingTestBase.h"
#include "core/editing/FrameSelection.h"
#include "core/editing/VisibleUnits.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLBodyElement.h"
......@@ -19,6 +23,7 @@
#include "core/html/HTMLHtmlElement.h"
#include "core/html/HTMLTextAreaElement.h"
#include "platform/heap/Handle.h"
#include "platform/testing/UnitTestHelpers.h"
#include "platform/wtf/Compiler.h"
#include "platform/wtf/RefPtr.h"
#include "platform/wtf/text/AtomicString.h"
......@@ -254,4 +259,41 @@ TEST_F(RangeTest, ToPosition) {
EXPECT_EQ(position, range.EndPosition());
}
static void LoadAhem(DummyPageHolder& page_holder, Document& document) {
RefPtr<SharedBuffer> shared_buffer =
testing::ReadFromFile(testing::CoreTestDataPath("Ahem.ttf"));
StringOrArrayBufferOrArrayBufferView buffer =
StringOrArrayBufferOrArrayBufferView::fromArrayBuffer(
DOMArrayBuffer::Create(shared_buffer));
FontFace* ahem =
FontFace::Create(&document, "Ahem", buffer, FontFaceDescriptors());
ScriptState* script_state =
ToScriptStateForMainWorld(&page_holder.GetFrame());
DummyExceptionStateForTesting exception_state;
FontFaceSet::From(document)->addForBinding(script_state, ahem,
exception_state);
}
TEST_F(RangeTest, BoundingRectMustIndependentFromSelection) {
LoadAhem(GetDummyPageHolder(), GetDocument());
GetDocument().body()->setInnerHTML(
"<div style='font: Ahem; width: 2em;letter-spacing: 5px;'>xx xx </div>");
Node* const div = GetDocument().QuerySelector("div");
// "x^x
// x|x "
Range* const range =
Range::Create(GetDocument(), div->firstChild(), 1, div->firstChild(), 4);
const FloatRect rect_before = range->BoundingRect();
EXPECT_GT(rect_before.Width(), 0);
EXPECT_GT(rect_before.Height(), 0);
Selection().SetSelection(SelectionInDOMTree::Builder()
.SetBaseAndExtent(EphemeralRange(range))
.Build());
GetDocument().View()->UpdateAllLifecyclePhases();
EXPECT_EQ(Selection().SelectedText(), "x x");
const FloatRect rect_after = range->BoundingRect();
EXPECT_EQ(rect_before, rect_after);
}
} // namespace blink
......@@ -293,7 +293,7 @@ static void UpdateLayoutObjectState(const SelectionPaintRange& new_range,
}
std::pair<int, int> LayoutSelection::SelectionStartEnd() {
Commit();
DCHECK(!HasPendingSelection());
if (paint_range_.IsNull())
return std::make_pair(-1, -1);
return std::make_pair(paint_range_.StartOffset(), paint_range_.EndOffset());
......@@ -348,6 +348,10 @@ static SelectionPaintRange CalcSelectionPaintRange(
end_layout_object, end_pos.ComputeEditingOffset());
}
void LayoutSelection::SetHasPendingSelection() {
has_pending_selection_ = true;
}
void LayoutSelection::Commit() {
if (!HasPendingSelection())
return;
......
......@@ -93,7 +93,7 @@ class LayoutSelection final : public GarbageCollected<LayoutSelection> {
}
bool HasPendingSelection() const { return has_pending_selection_; }
void SetHasPendingSelection() { has_pending_selection_ = true; }
void SetHasPendingSelection();
void Commit();
IntRect SelectionBounds();
......
......@@ -331,7 +331,9 @@ static FloatRect LocalQuadForTextBox(InlineTextBox* box,
unsigned start,
unsigned end) {
unsigned real_end = std::min(box->end() + 1, end);
LayoutRect r = box->LocalSelectionRect(start, real_end);
const bool include_newline_space_width = false;
LayoutRect r =
box->LocalSelectionRect(start, real_end, include_newline_space_width);
if (r.Height()) {
// Change the height and y position (or width and x for vertical text)
// because selectionRect uses selection-specific values.
......
......@@ -269,7 +269,10 @@ float InlineTextBox::NewlineSpaceWidth() const {
return style_to_use.GetFont().SpaceWidth();
}
LayoutRect InlineTextBox::LocalSelectionRect(int start_pos, int end_pos) const {
LayoutRect InlineTextBox::LocalSelectionRect(
int start_pos,
int end_pos,
bool consider_current_selection) const {
int s_pos = std::max(start_pos - start_, 0);
int e_pos = std::min(end_pos - start_, (int)len_);
......@@ -315,7 +318,7 @@ LayoutRect InlineTextBox::LocalSelectionRect(int start_pos, int end_pos) const {
top_point = LayoutPoint(r.X(), sel_top);
width = logical_width;
height = sel_height;
if (HasWrappedSelectionNewline()) {
if (consider_current_selection && HasWrappedSelectionNewline()) {
if (!IsLeftToRightDirection())
top_point.SetX(LayoutUnit(top_point.X() - NewlineSpaceWidth()));
width += NewlineSpaceWidth();
......@@ -326,7 +329,7 @@ LayoutRect InlineTextBox::LocalSelectionRect(int start_pos, int end_pos) const {
height = logical_width;
// TODO(wkorman): RTL text embedded in top-to-bottom text can create
// bottom-to-top situations. Add tests and ensure we handle correctly.
if (HasWrappedSelectionNewline())
if (consider_current_selection && HasWrappedSelectionNewline())
height += NewlineSpaceWidth();
}
......
......@@ -137,7 +137,10 @@ class CORE_EXPORT InlineTextBox : public InlineBox {
return LayoutRect(X(), Y(), Width(), Height());
}
virtual LayoutRect LocalSelectionRect(int start_pos, int end_pos) const;
virtual LayoutRect LocalSelectionRect(
int start_pos,
int end_pos,
bool include_newline_space_width = true) const;
bool IsSelected(int start_pos, int end_pos) const;
void SelectionStartEnd(int& s_pos, int& e_pos) const;
......
......@@ -134,8 +134,10 @@ FloatRect SVGInlineTextBox::SelectionRectForTextFragment(
return selection_rect;
}
LayoutRect SVGInlineTextBox::LocalSelectionRect(int start_position,
int end_position) const {
LayoutRect SVGInlineTextBox::LocalSelectionRect(
int start_position,
int end_position,
bool consider_current_selection) const {
int box_start = Start();
start_position = std::max(start_position - box_start, 0);
end_position = std::min(end_position - box_start, static_cast<int>(Len()));
......
......@@ -47,7 +47,8 @@ class SVGInlineTextBox final : public InlineTextBox {
LayoutUnit line_top,
LayoutUnit line_bottom) const override;
LayoutRect LocalSelectionRect(int start_position,
int end_position) const override;
int end_position,
bool = true) const override;
bool MapStartEndPositionsIntoFragmentCoordinates(const SVGTextFragment&,
int& start_position,
......
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