Bug 212856 – Text form controls can scroll by 1px when value is the same length as size. No scrolling should happen.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Text form controls scroll by 1px when value is the same length and size, which shouldn't happen.

[Megan Gardner]

Created attachment 401216 [details]
Patch

[Megan Gardner]

Created attachment 401218 [details]
Patch

[Megan Gardner]

Created attachment 401475 [details]
Patch

[Megan Gardner]

Created attachment 401512 [details]
Patch

[zalan]

I think this should match with the end padding adjustment in ComplexLineLayout::addOverflowFromInlineChildren(). Something along these lines:

diff --git a/Source/WebCore/rendering/ComplexLineLayout.cpp b/Source/WebCore/rendering/ComplexLineLayout.cpp
index 41bb52f06ef..7fb5e37c05d 100644
--- a/Source/WebCore/rendering/ComplexLineLayout.cpp
+++ b/Source/WebCore/rendering/ComplexLineLayout.cpp
@@ -2092,8 +2092,8 @@ void ComplexLineLayout::addOverflowFromInlineChildren()
 
     LayoutUnit endPadding = m_flow.hasOverflowClip() ? m_flow.paddingEnd() : 0_lu;
     // FIXME: Need to find another way to do this, since scrollbars could show when we don't want them to.
-    if (m_flow.hasOverflowClip() && !endPadding && m_flow.element() && m_flow.element()->isRootEditableElement() && style().isLeftToRightDirection())
-        endPadding = 1;
+    if (!endPadding)
+        endPadding = m_flow.endPaddingWidthForCaret();
     for (RootInlineBox* curr = firstRootBox(); curr; curr = curr->nextRootBox()) {
         m_flow.addLayoutOverflow(curr->paddedLayoutOverflowRect(endPadding));
         RenderFragmentContainer* fragment = m_flow.enclosingFragmentedFlow() ? curr->containingFragment() : nullptr;
diff --git a/Source/WebCore/rendering/RenderBlockFlow.h b/Source/WebCore/rendering/RenderBlockFlow.h
index b429b76921b..33cc0068017 100644
--- a/Source/WebCore/rendering/RenderBlockFlow.h
+++ b/Source/WebCore/rendering/RenderBlockFlow.h
@@ -403,6 +403,8 @@ public:
 
     void addFloatsToNewParent(RenderBlockFlow& toBlockFlow) const;
 
+    LayoutUnit endPaddingWidthForCaret() const;
+
 protected:
     bool shouldResetLogicalHeightBeforeLayout() const override { return true; }
 
@@ -652,6 +654,13 @@ inline LayoutIntegration::LineLayout* RenderBlockFlow::layoutFormattingContextLi
 {
     return hasLayoutFormattingContextLineLayout() ? WTF::get<std::unique_ptr<LayoutIntegration::LineLayout>>(m_lineLayout).get() : nullptr;
 }
+
+inline LayoutUnit RenderBlockFlow::endPaddingWidthForCaret() const
+{
+    if (element() && element()->isRootEditableElement() && hasOverflowClip() && style().isLeftToRightDirection())
+        return caretWidth;
+    return { };
+}
 #endif
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/RenderTextControlSingleLine.cpp b/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
index 16602173095..ebfad354652 100644
--- a/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
+++ b/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
@@ -306,7 +306,7 @@ float RenderTextControlSingleLine::getAverageCharWidth()
 
 LayoutUnit RenderTextControlSingleLine::preferredContentLogicalWidth(float charWidth) const
 {
-    int factor;
+    int factor = 0;
     bool includesDecoration = inputElement().sizeShouldIncludeDecoration(factor);
     if (factor <= 0)
         factor = 20;
@@ -334,6 +334,9 @@ LayoutUnit RenderTextControlSingleLine::preferredContentLogicalWidth(float charW
     if (includesDecoration)
         result += inputElement().decorationWidth();
 
+    if (auto* innerRenderer = innerTextElement()->renderer())
+        result += innerRenderer->endPaddingWidthForCaret();
+
     return result;
 }

[Wenson Hsieh]

Comment on attachment 401512 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401512&action=review

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:195
> -        self._interrupt_if_at_failure_limits(run_results)
> +

Please remember to revert this Tools/ change as well.

[zalan]

> +inline LayoutUnit RenderBlockFlow::endPaddingWidthForCaret() const
> +{
> +    if (element() && element()->isRootEditableElement() &&
> hasOverflowClip() && style().isLeftToRightDirection())
> +        return caretWidth;
> +    return { };
> +}
>  #endif
if (element() && element()->isRootEditableElement() && hasOverflowClip() && style().isLeftToRightDirection() && !paddingEnd())

[Megan Gardner]

Created attachment 401664 [details]
Patch

[Megan Gardner]

Created attachment 401681 [details]
Patch

[Megan Gardner]

Created attachment 401686 [details]
Patch

[Megan Gardner]

Created attachment 401693 [details]
Patch

[Megan Gardner]

Created attachment 401699 [details]
Patch

[Megan Gardner]

Created attachment 401708 [details]
Patch

[Megan Gardner]

Created attachment 401709 [details]
Patch

[Megan Gardner]

Created attachment 401712 [details]
Patch

[Megan Gardner]

Created attachment 401772 [details]
Patch

[Megan Gardner]

Created attachment 401793 [details]
Patch

[zalan]

Comment on attachment 401793 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401793&action=review

> Source/WebCore/rendering/RenderBlockFlow.h:662
> +    if (element() && element()->isRootEditableElement() && hasOverflowClip() && style().isLeftToRightDirection())
> +        return caretWidth;

I think this should include the "!paddingEnd()" check too (see Comment #7). -This also has an unfortunate side effect of having different renderer width when some (non-geometry related) style changes. Let's see if it causes any issues. We could always fall back to the simple, let's always add a padding approach.

[Megan Gardner]

Created attachment 401949 [details]
Patch

[Megan Gardner]

Created attachment 401957 [details]
Patch

[Megan Gardner]

Created attachment 401967 [details]
Patch for landing

[EWS]

Committed r263073: <https://trac.webkit.org/changeset/263073>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401967 [details].

[Radar WebKit Bug Importer]

<rdar://problem/64389038>

[Megan Gardner]

<rdar://problem/63541707>

[Truitt Savell]

Updated the expected file for tables/mozilla_expected_failures/bugs/bug2479-5.html in https://trac.webkit.org/changeset/263086/webkit