Fix arrow placement on menulist with padding

Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are fixed by this change.

Committed: https://crrev.com/c5e261c4f8e390e84d45ce6b62b4d434de354cac
Cr-Commit-Position: refs/heads/master@{#392008}

[Stephen White, 2016-05-02 14:11:41]

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

[Stephen White, 2016-05-02 14:11:41]

I'm not the right person to review this. tkent@, could you take a look?

[tkent, 2016-05-05 23:10:55]

tkent@chromium.org changed reviewers:
- senorblanco@chromium.org, vollick@chromium.org

[tkent, 2016-05-05 23:10:56]

The third paragraph of the CL description (First commit, ...) isn't suitable for
the CL description.  Please remove it.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:829: crbug.com/586127
fast/forms/select/menulist-appearance-basic.html [ NeedsManualRebaseline ]
NeedsManualRebaseline -> NeedsRebaseline
Because our rebaseline bot is running now.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:830: crbug.com/586127
fast/forms/select/basic-selects.html [ NeedsManualRebaseline ]
Ditto.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/ThemePainterDefault.cpp (right):

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/ThemePainterDefault.cpp:274: ? rect.x() +
arrowPadding * box.styleRef().effectiveZoom()
We need to add borderLeft() for RTL.
Also, we need to add a RTL + border test case to menulist-appearance-basic.html.

[AdamB, 2016-05-06 04:10:08]

Description was changed from

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

First commit, so please provide any feedback or corrections. I have
filled out the CLA. Trying to do a manual rebaseline (may misunderstand
this part).

BUG=586127
R=vollick@chromium.org,senorblanco@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.
==========

to

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.
==========

[AdamB, 2016-05-06 04:34:42]

Thank you for your feedback. I've made the recommended changes and run
webkit-tests locally to check for any reverts.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:829: crbug.com/586127
fast/forms/select/menulist-appearance-basic.html [ NeedsManualRebaseline ]
On 2016/05/05 23:10:56, tkent wrote:
> NeedsManualRebaseline -> NeedsRebaseline
> Because our rebaseline bot is running now.

Done.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/TestExpectations:830: crbug.com/586127
fast/forms/select/basic-selects.html [ NeedsManualRebaseline ]
On 2016/05/05 23:10:56, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/ThemePainterDefault.cpp (right):

https://codereview.chromium.org/1931273003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/ThemePainterDefault.cpp:274: ? rect.x() +
arrowPadding * box.styleRef().effectiveZoom()
On 2016/05/05 23:10:56, tkent wrote:
> We need to add borderLeft() for RTL.
> Also, we need to add a RTL + border test case to
menulist-appearance-basic.html.

Done and added tests. Confirmed that we did have same issue with RTL previous
and is now resolved.

[tkent, 2016-05-06 04:36:22]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-05-06 04:36:23]

lgtm

[commit-bot: I haz the power, 2016-05-06 04:36:29]

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

[commit-bot: I haz the power, 2016-05-06 05:37:08]

Description was changed from

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.
==========

to

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.
==========

[commit-bot: I haz the power, 2016-05-06 05:37:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-06 05:38:24]

Description was changed from

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.
==========

to

==========
Fix arrow placement on menulist with padding

The arrow calculation does not take padding into account, so it is
"underneath" the padding if >1.

BUG=586127
R=tkent@chromium.org
TEST=Have run the /fast/form tests, and nothing reverts except items which are
fixed by this change.

Committed: https://crrev.com/c5e261c4f8e390e84d45ce6b62b4d434de354cac
Cr-Commit-Position: refs/heads/master@{#392008}
==========

[commit-bot: I haz the power, 2016-05-06 05:38:25]

Patchset 2 (id:??) landed as
https://crrev.com/c5e261c4f8e390e84d45ce6b62b4d434de354cac
Cr-Commit-Position: refs/heads/master@{#392008}