Closed Bug 139989 Opened 23 years ago Closed 23 years ago

[forms.css] Fieldset needs prettier default border

Categories

(Core :: Layout: Form Controls, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: sairwas, Assigned: vhaarr+bmo)

Details

(Keywords: html4, polish)

Attachments

(5 files)

Currently (in 1.0 RC1), the fieldset element has the following border style in forms.css: border: 2px solid; ...which is rather ugly and inconsistent with other fieldset-supporting browsers. I would suggest something like this instead: border: 2px groove threedface;
Keywords: html4
-> Form Controls.
Assignee: dbaron → rods
Component: Style System → HTML Form Controls
QA Contact: ian → tpreston
Exactly. The padding should also be changed from `4px 4px 4px 4px' to `0.7em 0.67em', which is a scalable equivalent to the `12px 10px' shown in <http://developer.apple.com/techpubs/mac/HIGOS8Guide/thig-52.html#HEADING52-58>.
Keywords: polish
This patch changes the border to be 'groove threedface', and also changes the padding to use 'em', in order to make it scale according to different font-sizes. I just did it quick, and copied the original forms.css to forms2.css, and diff -wu'ed them.. If that causes a problem, please state so.
(Just for the record, my `0.67em' was a miscalculation, and the `0.625em' in the patch is correct.)
Note that legend has currently pixel-sized padding values as well: padding-left: 2px; padding-right: 2px; This should probably be corrected as well while you're at it...
I don't wan't to be CC'ed on the mpt class bugs ;-), this is UI, I have no clue of UI design and I dont wan't to have one.
Attached patch Removed the ','Splinter Review
This patch removes the , between the paddings.
Antti: That's not padding, that's margin. Your eyes decieve you :) Margin is the box's distance to the outer content, while padding is the box's contents distance to the box's edges. http://www.w3.org/TR/REC-CSS2/box.html#mpb-examples Did that make sense ? :)
Hm, this is what http://developer.apple.com/techpubs/mac/HIGOS8Guide/thig-52.html#HEADING52-58 has to say about group box titles (legends): "There should be at least 3 pixels on each side of the title text." But at the same time, the legend should be left-aligned with the fieldset contents: http://developer.apple.com/techpubs/mac/HIGOS8Guide/graphics/HID_L-20.gif *If* we decide to implement that, just hacking forms.css doesn't seem to be enough...
Antti: Right, this has nothing to do with the heading. This is about the /content/ of the box. With the change done here, the distance from the content to the box's edges will be dynamic, based on the font size.
Attachment #80928 - Flags: review+
reassigning to Mabus
Assignee: rods → mabus
Fix checked into trunk by axel@pike.org. Marking RESOLVED FIXED.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
This bug was reported against an RC1 (branch) build. I suggest re-opening this bug, and requesting driver approval... Cosmetic fixes like this should go in to releases if it is going to be the default down the line... A screenshot of before and after would be nice also. :)
Reopening, email on the way to drivers@mozilla.org requesting approval. Lets cross our fingers.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Have you tested that the LEGENDs are still positioned correctly relative to the fieldset at different font sizes? (i.e., that there aren't any hardcoded sizes in the fieldset/legend code that the change to the padding would mess up?) Testcases attached to the bug would be nice.
No, I haven't, actually. But I won't get to this untill Monday, at the earliest. If someone else could provide testcases, that would be very appreciated. If not, I'll attach some as soon as I can. I just took a brief look through the code in forms.css, and it seems the dynamic padding will not affect the legend elements padding. But then again, I could be wrong. In fact, I don't know the correct behavior.. should the legend element change padding (ie, follow the fieldset element), or should it stay put as the fieldset font-size change ?
Attached image Screenshot before
Attached image Screenshot after
Attached "before" and "after" screenshots, taken on 1.0 RC1, Win32, small fonts, default settings.
Attached file Test case
> Have you tested that the LEGENDs are still positioned correctly relative to > the fieldset at different font sizes? That was my exact concern in comment #5... But it seems that legends are not positioned in any meaningful way to begin with (see the "before" screenshot), so our patch doesn't really make things worse. As I stated, http://developer.apple.com/techpubs/mac/HIGOS8Guide/graphics/HID_L-20.gif has a nice spec for this, but implementing that would probably require some changes to the fieldset rendering code. Anyway, I feel that the legend issue can be taken to another bug.
Great work, Antti ! I have emailed drivers@mozilla.org now, and we will hopefully get a review soon. Sorry, I just re-read comment #5, and realize that my response in comment #8 really had nothing to do with your comment :) sorry. I thought you were talking about the fieldset element.
Comment on attachment 80928 [details] [diff] [review] Removed the ',' a=dbaron for 1.0 branch checkin
Attachment #80928 - Flags: approval+
Patch 80928 checked into the 1.0 branch by timeless@mac.com Marking RESOLVED FIXED.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Keywords: fixed1.0.0
Verified Mac OS X branch build 2002052803, Win 2k branch build 2002052808 and linux branch branch build 20020522
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: