381639 - [Mac] Aqua <button> text should not move when button is :active

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P4)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Load data:text/html,<button>foo</button>
2. Hold the mouse down on the button.

Result: "foo" moves right a pixel.

Expected: "foo" should not move, for consistency with Aqua widgets in other Mac apps.

Comment 1

[philippe (part-time)]

I think this is closely related to bug 375436

Comment 2

[monkeypox37]

Attachment: unmodified forms.css

Comment 3

[monkeypox37]

Attachment: modified forms.css

Comment 4

[monkeypox37]

Attachment: safari version for comparison

Comment 5

[monkeypox37]

This can actually be fixed by modifying the forms.css file. I was going to post this in a new bug but this can be fixed at the same time so may as well piggyback here.

The padding for buttons in forms.css is set to 0px 6px 0px 6px for the normal state and 0px 5px 0px 7px for the clicked (active:hover) state. The difference is why it's shifting a pixel.

It's also set too large (which was going to be the bug I filed), IMO. If you set it to 0px 2px 0px 2px then it more closely matches the output you get with Safari (though the button shape is a bit different and Firefox adds the focus thing which is cut off, but whatever) which fixes the case where the buttons are too large and they spill over onto a new line in a form.

I'll attach some screenshots that show the unpatched, patched and Safari views.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment 5 also sounds a bit like the issue in bug 382147.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Is the css change in comment 5 the correct way to fix this bug, or should it really be fixed in widget/native theme code?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It should be fixed in native theme code -- buttons should override the padding in GetWidgetPadding.  Unthemed buttons are supposed to move when clicked.

Comment 9

[Josh Aas]

Attachment: possible fix v1.0

We probably want something like this, but I haven't messed with it enough to fully endorse this and request review.

Comment 10

[Josh Aas]

Attachment: fix v1.1

Comment 11

[Colin Barrett [:cbarrett]]

Comment on attachment 288959 [details] [diff] [review]
fix v1.1

Update the comment in GetWidgetBorder and add one in GetWidgetPadding explaining why we're doing the override.

r=cbarrett with that.

Comment 12

[Josh Aas]

landed on trunk

Comment 13

[Josh Aas]

This fixed reftest failure, bug 381835.