Closed Bug 394127 Opened 17 years ago Closed 17 years ago

Aqua textfields - textarea, input type="text" - do not accept padding.

Categories

(Core :: Widget: Cocoa, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: phiw2, Assigned: jaas)

References

Details

(Keywords: regression, testcase)

Attachments

(3 files)

Attached file test case (deleted) —
Both textarea and one-line textfields styled only with padding by the author. The padding is not applied. If more styling is applied by the author (border, background, ...) then it works correctly). FX 2.0.0.x Windows with themed widgets does apply the padding. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a8pre) Gecko/2007082821 Minefield/3.0a8pre
Attached image screenshot (deleted) —
Minefield (left) and latest WebKit build (right).
Attached patch fix v1.0 (deleted) — Splinter Review
This fixes the padding problem and it also fixes the floating scrollbars in textareas which are nasty. Aqua text fields should have their text contents padded more though (especially on the left and right). Unfortunately we can't do that right now without bringing in the scrollbars as well, which leads to the floating scrollbars problem (ew). Fixing this will probably require some non-trivial work on the way we lay out textareas. This patch is undoubtedly the right thing to do now even though it seems to regress the exact placement of text within the control because the floating scrollbars problem is way worse than the text having a little less padding than it should. Windows and Linux don't require this special text placement, so not being able to do it never mattered until now.
Attachment #278937 - Flags: review?(cbarrett)
Flags: blocking1.9+
Comment on attachment 278937 [details] [diff] [review] fix v1.0 I tested this with data:text/html,<input type=text> and noticed that there was a rendering difference -- the patched version had a little bit taller textfield. Might be OK, (if that's what the markup says in forms.css) Find out though. >and only use this to return PR_TRUE >+// if you want to stop CSS padding values from being taken into account. Should that be "and only return PR_TRUE"? The wording right now is a little confusing.
Normal aqua text fields are 22 pixels tall, Safari text fields are 19 pixels tall. Before my patch we had a height of 21 and with my patch we have a height of 19. The padding change comes from the now-enabled padding values in forms.css (padding: 1px 0 1px 0;). I think we should be going for the smaller control, like Safari does. Thanks for catching that though, I didn't really intend to do it.
Comment on attachment 278937 [details] [diff] [review] fix v1.0 fix the wording on checkin, r=me
Attachment #278937 - Flags: review?(cbarrett) → review+
Flags: in-testsuite?
This patch works fine and the behaviour is now consistent with Fx 2.0 Win and WebKit (can't test Minefield Win - my VPC install is way too slow to go through the ring ring dance of nightly updates). And is consistent with most OS X browsers. For the record, the textarea problem Josh mentioned in comment 2 was originally filed as bug 382143. That floating scrollbar in textareas isn't pretty but it happens on Gecko/all platforms and all browsers on OS X except Opera. That is meat for another bug.
Attachment #278937 - Flags: superreview?(roc)
Attachment #278937 - Flags: superreview?(roc) → superreview+
landed on trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
verified with Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a8pre) Gecko/2007090316 Minefield/3.0a8pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: