INPUT textfield: Do not remove line-height of innerEditor if we don't need to remove it.

INPUT textfield: Do not remove line-height of innerEditor if we don't need to remove it.

The root cause of crbug.com/601209 is a difference between
 - text centering behavior of taller line-height, and
 - innerEditor box centering behavior in LayoutTextControlSingleLine.

It's hard to unify them.  So this CL updates LayoutTextControlSingleLine::
createInnerEditorStyle so that line-height is not removed if we don't need to
remove it.

BUG=601209
Committed: https://crrev.com/5418e1dbff8bdbea12ea0e55c8dc8828fdbbd820
Cr-Commit-Position: refs/heads/master@{#389767}

[tkent, 2016-04-26 02:04:21]

Description was changed from

==========
INPUT textfield: Do not remove line-height of innerEditor if we don't need to
remove it.

The root cause of crbug.com/601209 is a difference between
 - text centering behavior of taller line-height, and
 - initial line-height box centering behavior in LayoutTextControlSingleLine.

It's hard to unify them.  So this CL updates LayoutTextControlSingleLine::
createInnerEditorStyle so that line-height is not removed if we don't need to
remove it.

BUG=601209
==========

to

==========
INPUT textfield: Do not remove line-height of innerEditor if we don't need to
remove it.

The root cause of crbug.com/601209 is a difference between
 - text centering behavior of taller line-height, and
 - innerEditor box centering behavior in LayoutTextControlSingleLine.

It's hard to unify them.  So this CL updates LayoutTextControlSingleLine::
createInnerEditorStyle so that line-height is not removed if we don't need to
remove it.

BUG=601209
==========

[tkent, 2016-04-26 04:13:36]

tkent@chromium.org changed reviewers:
+ kojii@chromium.org

[tkent, 2016-04-26 04:13:36]

kojii@, would you review this please?

fast/forms/placeholder-position.html is failing on try bots.  It just needs
rebaseline, and I'll add it to TestExpectations before landing.

[kojii, 2016-04-26 06:03:21]

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp
(right):

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp:288: if
(logicalHeight.isFixed() && logicalHeight.getFloatValue() > computedLineHeight)
Are Percent and Calculated converted to Fixed at this point? By changing
!isIntrinsicOrAuto() to isFixed(), these are left in between, correct?

[tkent, 2016-04-26 07:03:28]

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp
(right):

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp:288: if
(logicalHeight.isFixed() && logicalHeight.getFloatValue() > computedLineHeight)
On 2016/04/26 at 06:03:21, kojii wrote:
> Are Percent and Calculated converted to Fixed at this point?

No.  We are unable to determine if it's ok to remove line-height.  So, we'll see
the editing-text-cutting-issues again in percent/calculated height.

Another choice is to remove line-height if percent/calculated height. Like
   if (logicalHeight.hasPercent() || logicalHeight.isFixed() && ...)

[tkent, 2016-04-26 07:44:49]

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp
(right):

https://codereview.chromium.org/1915353003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp:288: if
(logicalHeight.isFixed() && logicalHeight.getFloatValue() > computedLineHeight)
On 2016/04/26 at 07:03:28, tkent wrote:
> On 2016/04/26 at 06:03:21, kojii wrote:
> > Are Percent and Calculated converted to Fixed at this point?
> 
> No.  We are unable to determine if it's ok to remove line-height.  So, we'll
see the editing-text-cutting-issues again in percent/calculated height.
> 
> Another choice is to remove line-height if percent/calculated height. Like
>    if (logicalHeight.hasPercent() || logicalHeight.isFixed() && ...)

We discussed offline, and agreed that we should not change the behavior in a
case of Percent/Calculated.  I have uploaded new patch set.

[kojii, 2016-04-26 07:50:48]

lgtm

[tkent, 2016-04-26 13:13:55]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-04-26 13:13:55]

The patchset sent to the CQ was uploaded after l-g-t-m from kojii@chromium.org
Link to the patchset: https://codereview.chromium.org/1915353003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-26 13:14:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915353003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915353003/40001

[commit-bot: I haz the power, 2016-04-26 13:17:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-26 13:18:57]

Description was changed from

==========
INPUT textfield: Do not remove line-height of innerEditor if we don't need to
remove it.

The root cause of crbug.com/601209 is a difference between
 - text centering behavior of taller line-height, and
 - innerEditor box centering behavior in LayoutTextControlSingleLine.

It's hard to unify them.  So this CL updates LayoutTextControlSingleLine::
createInnerEditorStyle so that line-height is not removed if we don't need to
remove it.

BUG=601209
==========

to

==========
INPUT textfield: Do not remove line-height of innerEditor if we don't need to
remove it.

The root cause of crbug.com/601209 is a difference between
 - text centering behavior of taller line-height, and
 - innerEditor box centering behavior in LayoutTextControlSingleLine.

It's hard to unify them.  So this CL updates LayoutTextControlSingleLine::
createInnerEditorStyle so that line-height is not removed if we don't need to
remove it.

BUG=601209

Committed: https://crrev.com/5418e1dbff8bdbea12ea0e55c8dc8828fdbbd820
Cr-Commit-Position: refs/heads/master@{#389767}
==========

[commit-bot: I haz the power, 2016-04-26 13:18:58]

Patchset 3 (id:??) landed as
https://crrev.com/5418e1dbff8bdbea12ea0e55c8dc8828fdbbd820
Cr-Commit-Position: refs/heads/master@{#389767}