If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Make the size of XUL radio buttons and checkboxes dependent on the font size

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Toolkit
Themes
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
mozilla1.9.2a1
All
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 386510 [details] [diff] [review]
patch v1

(This is the radio/checkbox counterpart to bug 482681.)

Right now, radio buttons and checkboxes are always drawn in the "regular" size, 16x16, regardless of the font size. The attached patch fixes that.

The natural sizes of radios / checkboxes are:
 - mini: 11px
 - small: 13px
 - regular: 16px

If the native theme code is asked to draw a control with a non-natural size, it snaps to the next smaller natural size.

The corresponding font sizes are:
 - mini: 9px
 - small: 11px
 - regular: 13px

The number 1.3 satisfies the following conditions:
 - 11px <= floor(1.3 *  9px) <= ceil(1.3 *  9px) <= 12px --> mini
 - 13px <= floor(1.3 * 11px) <= ceil(1.3 * 11px) <= 15px --> small
 - 16px <= floor(1.3 * 13px) <= ceil(1.3 * 13px) <= 17px --> regular
Attachment #386510 - Flags: review?(dao)
(Assignee)

Comment 1

8 years ago
Created attachment 386512 [details]
before
(Assignee)

Comment 2

8 years ago
Created attachment 386513 [details]
after
(Assignee)

Comment 3

8 years ago
Created attachment 386597 [details] [diff] [review]
v1.1

with missing reftest.list addition
Attachment #386510 - Attachment is obsolete: true
Attachment #386597 - Flags: review?(dao)
Attachment #386510 - Flags: review?(dao)
What does vertical-align:middle achieve here?
(Assignee)

Comment 5

8 years ago
If I leave it out, the controls are 1px too low when the font is small or mini.
That doesn't fully solve it, does it? I'm looking at [_] Priority 3 and [_] Show Source in attachment 386513 [details].
(Assignee)

Comment 7

8 years ago
You're right, the system not bulletproof. I think the problem is related to the way the control is centered in its pixel-snapped rect by the native theme code.
I can look at that at some point again, but I don't think it's worth the hassle right now.
Ok, but please file a follow-up bug and add the bug number to these two lines.
Btw, have you tested this with multi-line labels? I'm still not sure if vertical-align is the right tool for this at all.
(Assignee)

Comment 10

8 years ago
I remember now why vertical-align makes a difference: In bug 394892, I made native theming check the value of vertical-align when deciding in which part of the draw rect it should position the snapped control.
I've filed bug 503833 for the pixel rounding issue.
Multi-line labels work fine.
(Assignee)

Comment 11

8 years ago
Created attachment 388518 [details] [diff] [review]
v2

This patch works better, it's right in more cases. I can't really justify all of the changes, though; but I don't think it's worth spending more time on these barely-noticeable pixel offsets.
Attachment #386597 - Attachment is obsolete: true
Attachment #388518 - Flags: review?(dao)
Attachment #386597 - Flags: review?(dao)

Updated

8 years ago
Attachment #388518 - Flags: review?(dao) → review+
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/mozilla-central/rev/29fb65d31455
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Comment 13

8 years ago
Backed out:
http://hg.mozilla.org/mozilla-central/rev/ae89cfbc5427
http://hg.mozilla.org/mozilla-central/rev/38bc9063e689

test_contextmenu_list.xul had one failure. I suppose the test needs to be changed to round the bounding client rect position, but I don't have any time to monitor the tree now.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1247930309.1247932698.29434.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

8 years ago
Created attachment 389467 [details] [diff] [review]
fix test
Attachment #389467 - Flags: review?(enndeakin)

Updated

8 years ago
Attachment #389467 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 15

8 years ago
http://hg.mozilla.org/mozilla-central/rev/3057e39a3cb4
http://hg.mozilla.org/mozilla-central/rev/bb66c5956deb
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.