Commit c40b6f3f authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

TextControl NG: Fix '::selection' invalidation

* Share the logic of LayoutTextControl::StyleDidChange() by making it
  a static member function.

* Make LayoutTextControlTest a parameterized test for LayoutNGTextArea
  flag and LayoutNGTextField flag.

This CL has no behavior changes yet.

Bug: 1040826
Change-Id: I290bf46c86219052eaf9133d3382ff769e56a47e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2484039
Commit-Queue: Kent Tamura <tkent@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818403}
parent 0e1cda6a
...@@ -54,7 +54,13 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff, ...@@ -54,7 +54,13 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff,
const ComputedStyle* old_style) { const ComputedStyle* old_style) {
NOT_DESTROYED(); NOT_DESTROYED();
LayoutBlockFlow::StyleDidChange(diff, old_style); LayoutBlockFlow::StyleDidChange(diff, old_style);
TextControlInnerEditorElement* inner_editor = InnerEditorElement(); StyleDidChange(InnerEditorElement(), old_style, StyleRef());
}
// static
void LayoutTextControl::StyleDidChange(HTMLElement* inner_editor,
const ComputedStyle* old_style,
const ComputedStyle& new_style) {
if (!inner_editor) if (!inner_editor)
return; return;
LayoutBlock* inner_editor_layout_object = LayoutBlock* inner_editor_layout_object =
...@@ -65,7 +71,7 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff, ...@@ -65,7 +71,7 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff,
// (See TextControlInnerEditorElement::CreateInnerEditorStyle()). // (See TextControlInnerEditorElement::CreateInnerEditorStyle()).
{ {
StyleEngine::AllowMarkStyleDirtyFromRecalcScope scope( StyleEngine::AllowMarkStyleDirtyFromRecalcScope scope(
GetDocument().GetStyleEngine()); inner_editor->GetDocument().GetStyleEngine());
inner_editor->SetNeedsStyleRecalc( inner_editor->SetNeedsStyleRecalc(
kLocalStyleChange, kLocalStyleChange,
StyleChangeReasonForTracing::Create(style_change_reason::kControl)); StyleChangeReasonForTracing::Create(style_change_reason::kControl));
...@@ -75,7 +81,7 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff, ...@@ -75,7 +81,7 @@ void LayoutTextControl::StyleDidChange(StyleDifference diff,
// (see: GetUncachedSelectionStyle in SelectionPaintingUtils.cpp) so ensure // (see: GetUncachedSelectionStyle in SelectionPaintingUtils.cpp) so ensure
// the inner editor selection is invalidated anytime style changes and a // the inner editor selection is invalidated anytime style changes and a
// ::selection style is or was present on LayoutTextControl. // ::selection style is or was present on LayoutTextControl.
if (StyleRef().HasPseudoElementStyle(kPseudoIdSelection) || if (new_style.HasPseudoElementStyle(kPseudoIdSelection) ||
(old_style && old_style->HasPseudoElementStyle(kPseudoIdSelection))) { (old_style && old_style->HasPseudoElementStyle(kPseudoIdSelection))) {
inner_editor_layout_object->InvalidateSelectedChildrenOnStyleChange(); inner_editor_layout_object->InvalidateSelectedChildrenOnStyleChange();
} }
......
...@@ -49,6 +49,9 @@ class CORE_EXPORT LayoutTextControl : public LayoutBlockFlow { ...@@ -49,6 +49,9 @@ class CORE_EXPORT LayoutTextControl : public LayoutBlockFlow {
return true; return true;
} }
static void StyleDidChange(HTMLElement* inner_editor,
const ComputedStyle* old_style,
const ComputedStyle& new_style);
static int ScrollbarThickness(const LayoutBox& box); static int ScrollbarThickness(const LayoutBox& box);
static float GetAvgCharWidth(const ComputedStyle& style); static float GetAvgCharWidth(const ComputedStyle& style);
static bool HasValidAvgCharWidth(const Font& font); static bool HasValidAvgCharWidth(const Font& font);
......
...@@ -7,12 +7,19 @@ ...@@ -7,12 +7,19 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "third_party/blink/renderer/core/html/forms/text_control_element.h" #include "third_party/blink/renderer/core/html/forms/text_control_element.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h" #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
namespace blink { namespace blink {
namespace { namespace {
class LayoutTextControlTest : public RenderingTest { class LayoutTextControlTest : public testing::WithParamInterface<bool>,
public RenderingTest {
public:
LayoutTextControlTest()
: scoped_text_area_flag_(GetParam()),
scoped_text_field_flag_(GetParam()) {}
protected: protected:
TextControlElement* GetTextControlElementById(const char* id) { TextControlElement* GetTextControlElementById(const char* id) {
return To<TextControlElement>(GetDocument().getElementById(id)); return To<TextControlElement>(GetDocument().getElementById(id));
...@@ -42,9 +49,15 @@ class LayoutTextControlTest : public RenderingTest { ...@@ -42,9 +49,15 @@ class LayoutTextControlTest : public RenderingTest {
UpdateAllLifecyclePhasesForTest(); UpdateAllLifecyclePhasesForTest();
EXPECT_FALSE(selected_text.ShouldInvalidateSelection()); EXPECT_FALSE(selected_text.ShouldInvalidateSelection());
} }
private:
ScopedLayoutNGTextAreaForTest scoped_text_area_flag_;
ScopedLayoutNGTextFieldForTest scoped_text_field_flag_;
}; };
TEST_F(LayoutTextControlTest, INSTANTIATE_TEST_SUITE_P(All, LayoutTextControlTest, testing::Bool());
TEST_P(LayoutTextControlTest,
ChangingPseudoSelectionStyleShouldInvalidateSelectionSingle) { ChangingPseudoSelectionStyleShouldInvalidateSelectionSingle) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -61,7 +74,7 @@ TEST_F(LayoutTextControlTest, ...@@ -61,7 +74,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, TEST_P(LayoutTextControlTest,
ChangingPseudoSelectionStyleShouldInvalidateSelectionMulti) { ChangingPseudoSelectionStyleShouldInvalidateSelectionMulti) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -78,7 +91,7 @@ TEST_F(LayoutTextControlTest, ...@@ -78,7 +91,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, TEST_P(LayoutTextControlTest,
AddingPseudoSelectionStyleShouldInvalidateSelectionSingle) { AddingPseudoSelectionStyleShouldInvalidateSelectionSingle) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -94,7 +107,7 @@ TEST_F(LayoutTextControlTest, ...@@ -94,7 +107,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, TEST_P(LayoutTextControlTest,
AddingPseudoSelectionStyleShouldInvalidateSelectionMulti) { AddingPseudoSelectionStyleShouldInvalidateSelectionMulti) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -110,7 +123,7 @@ TEST_F(LayoutTextControlTest, ...@@ -110,7 +123,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, TEST_P(LayoutTextControlTest,
RemovingPseudoSelectionStyleShouldInvalidateSelectionSingle) { RemovingPseudoSelectionStyleShouldInvalidateSelectionSingle) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -126,7 +139,7 @@ TEST_F(LayoutTextControlTest, ...@@ -126,7 +139,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, TEST_P(LayoutTextControlTest,
RemovingPseudoSelectionStyleShouldInvalidateSelectionMulti) { RemovingPseudoSelectionStyleShouldInvalidateSelectionMulti) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<style> <style>
...@@ -142,7 +155,7 @@ TEST_F(LayoutTextControlTest, ...@@ -142,7 +155,7 @@ TEST_F(LayoutTextControlTest,
CheckSelectionInvalidationChanges(*selected_text); CheckSelectionInvalidationChanges(*selected_text);
} }
TEST_F(LayoutTextControlTest, HitTestSearchInput) { TEST_P(LayoutTextControlTest, HitTestSearchInput) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
<input id="input" type="search" <input id="input" type="search"
style="border-width: 20px; font-size: 30px; padding: 0"> style="border-width: 20px; font-size: 30px; padding: 0">
......
...@@ -12,11 +12,23 @@ namespace blink { ...@@ -12,11 +12,23 @@ namespace blink {
LayoutNGTextControlMultiLine::LayoutNGTextControlMultiLine(Element* element) LayoutNGTextControlMultiLine::LayoutNGTextControlMultiLine(Element* element)
: LayoutNGBlockFlow(element) {} : LayoutNGBlockFlow(element) {}
HTMLElement* LayoutNGTextControlMultiLine::InnerEditorElement() const {
return To<TextControlElement>(GetNode())->InnerEditorElement();
}
bool LayoutNGTextControlMultiLine::IsOfType(LayoutObjectType type) const { bool LayoutNGTextControlMultiLine::IsOfType(LayoutObjectType type) const {
return type == kLayoutObjectNGTextControlMultiLine || return type == kLayoutObjectNGTextControlMultiLine ||
LayoutNGBlockFlow::IsOfType(type); LayoutNGBlockFlow::IsOfType(type);
} }
void LayoutNGTextControlMultiLine::StyleDidChange(
StyleDifference style_diff,
const ComputedStyle* old_style) {
LayoutNGBlockFlow::StyleDidChange(style_diff, old_style);
LayoutTextControl::StyleDidChange(InnerEditorElement(), old_style,
StyleRef());
}
bool LayoutNGTextControlMultiLine::NodeAtPoint( bool LayoutNGTextControlMultiLine::NodeAtPoint(
HitTestResult& result, HitTestResult& result,
const HitTestLocation& hit_test_location, const HitTestLocation& hit_test_location,
...@@ -30,8 +42,7 @@ bool LayoutNGTextControlMultiLine::NodeAtPoint( ...@@ -30,8 +42,7 @@ bool LayoutNGTextControlMultiLine::NodeAtPoint(
if (stop_node && stop_node->NodeForHitTest() == result.InnerNode()) if (stop_node && stop_node->NodeForHitTest() == result.InnerNode())
return true; return true;
HTMLElement* inner_editor = HTMLElement* inner_editor = InnerEditorElement();
To<TextControlElement>(GetNode())->InnerEditorElement();
if (result.InnerNode() == GetNode() || result.InnerNode() == inner_editor) { if (result.InnerNode() == GetNode() || result.InnerNode() == inner_editor) {
LayoutTextControl::HitInnerEditorElement( LayoutTextControl::HitInnerEditorElement(
*this, *inner_editor, result, hit_test_location, accumulated_offset); *this, *inner_editor, result, hit_test_location, accumulated_offset);
......
...@@ -15,6 +15,8 @@ class LayoutNGTextControlMultiLine final : public LayoutNGBlockFlow { ...@@ -15,6 +15,8 @@ class LayoutNGTextControlMultiLine final : public LayoutNGBlockFlow {
explicit LayoutNGTextControlMultiLine(Element* element); explicit LayoutNGTextControlMultiLine(Element* element);
private: private:
HTMLElement* InnerEditorElement() const;
bool IsOfType(LayoutObjectType) const override; bool IsOfType(LayoutObjectType) const override;
const char* GetName() const override { const char* GetName() const override {
...@@ -27,6 +29,8 @@ class LayoutNGTextControlMultiLine final : public LayoutNGBlockFlow { ...@@ -27,6 +29,8 @@ class LayoutNGTextControlMultiLine final : public LayoutNGBlockFlow {
return true; return true;
} }
void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
bool NodeAtPoint(HitTestResult& result, bool NodeAtPoint(HitTestResult& result,
const HitTestLocation& hit_test_location, const HitTestLocation& hit_test_location,
const PhysicalOffset& accumulated_offset, const PhysicalOffset& accumulated_offset,
......
...@@ -4,14 +4,29 @@ ...@@ -4,14 +4,29 @@
#include "third_party/blink/renderer/core/layout/ng/layout_ng_text_control_single_line.h" #include "third_party/blink/renderer/core/layout/ng/layout_ng_text_control_single_line.h"
#include "third_party/blink/renderer/core/html/forms/text_control_element.h"
#include "third_party/blink/renderer/core/layout/layout_text_control.h"
namespace blink { namespace blink {
LayoutNGTextControlSingleLine::LayoutNGTextControlSingleLine(Element* element) LayoutNGTextControlSingleLine::LayoutNGTextControlSingleLine(Element* element)
: LayoutNGBlockFlow(element) {} : LayoutNGBlockFlow(element) {}
HTMLElement* LayoutNGTextControlSingleLine::InnerEditorElement() const {
return To<TextControlElement>(GetNode())->InnerEditorElement();
}
bool LayoutNGTextControlSingleLine::IsOfType(LayoutObjectType type) const { bool LayoutNGTextControlSingleLine::IsOfType(LayoutObjectType type) const {
return type == kLayoutObjectNGTextControlSingleLine || return type == kLayoutObjectNGTextControlSingleLine ||
LayoutNGBlockFlow::IsOfType(type); LayoutNGBlockFlow::IsOfType(type);
} }
void LayoutNGTextControlSingleLine::StyleDidChange(
StyleDifference style_diff,
const ComputedStyle* old_style) {
LayoutNGBlockFlow::StyleDidChange(style_diff, old_style);
LayoutTextControl::StyleDidChange(InnerEditorElement(), old_style,
StyleRef());
}
} // namespace blink } // namespace blink
...@@ -15,6 +15,8 @@ class LayoutNGTextControlSingleLine final : public LayoutNGBlockFlow { ...@@ -15,6 +15,8 @@ class LayoutNGTextControlSingleLine final : public LayoutNGBlockFlow {
explicit LayoutNGTextControlSingleLine(Element* element); explicit LayoutNGTextControlSingleLine(Element* element);
private: private:
HTMLElement* InnerEditorElement() const;
bool IsOfType(LayoutObjectType) const override; bool IsOfType(LayoutObjectType) const override;
const char* GetName() const override { const char* GetName() const override {
...@@ -26,6 +28,8 @@ class LayoutNGTextControlSingleLine final : public LayoutNGBlockFlow { ...@@ -26,6 +28,8 @@ class LayoutNGTextControlSingleLine final : public LayoutNGBlockFlow {
NOT_DESTROYED(); NOT_DESTROYED();
return true; return true;
} }
void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
}; };
} // namespace blink } // namespace blink
......
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