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

RESOLVED FIXED

Status

()

Core
Widget: Cocoa
P4
minor
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: Josh Aas)

Tracking

({testcase})

Trunk
x86
Mac OS X
testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
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

11 years ago
I think this is closely related to bug 375436

Comment 2

11 years ago
Created attachment 266628 [details]
unmodified forms.css

Comment 3

11 years ago
Created attachment 266629 [details]
modified forms.css

Comment 4

11 years ago
Created attachment 266630 [details]
safari version for comparison

Comment 5

11 years ago
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.
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?
It should be fixed in native theme code -- buttons should override the padding in GetWidgetPadding.  Unthemed buttons are supposed to move when clicked.
(Assignee)

Updated

11 years ago
Flags: blocking1.9?
(Assignee)

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
(Assignee)

Updated

11 years ago
Target Milestone: --- → mozilla1.9 M11
(Assignee)

Updated

11 years ago
Priority: -- → P4
Target Milestone: mozilla1.9 M11 → ---
(Assignee)

Comment 9

11 years ago
Created attachment 288445 [details] [diff] [review]
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.
(Assignee)

Comment 10

11 years ago
Created attachment 288959 [details] [diff] [review]
fix v1.1
Attachment #288445 - Attachment is obsolete: true
Attachment #288959 - Flags: review?(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.
Attachment #288959 - Flags: review?(cbarrett) → review+
(Assignee)

Updated

11 years ago
Attachment #288959 - Flags: superreview?(roc)
Attachment #288959 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 12

11 years ago
landed on trunk
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

11 years ago
This fixed reftest failure, bug 381835.
Blocks: 381835
Depends on: 844948
You need to log in before you can comment on or make changes to this bug.