Bug 22689

Summary: RenderThemeWin should match Firefox button metrics
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, hyatt
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Windows XP   
Attachments:
Description Flags
Sets the padding in RenderThemeWin
hyatt: review-
Sets padding only if style->appearance() != NoControlPart hyatt: review+

Description Ojan Vafai 2008-12-05 12:32:41 PST
RenderThemeWin should match one of Firefox or IE for compatibility sake. IE has a padding that is proportional to the amount of text in the button, which is crazy. So, matching Firefox seems best here.

Firefox has a padding:1px 3px on the inner element of buttons. This is in addition to any padding set on the element. So, padding:0 set in the CSS still has that extra few pixels in the actual size of the button, but still returns padding:0 from getComputedStyle.
Comment 1 Ojan Vafai 2008-12-05 16:18:49 PST
Created attachment 25799 [details]
Sets the padding in RenderThemeWin
Comment 2 Eric Seidel (no email) 2008-12-08 12:36:14 PST
Comment on attachment 25799 [details]
Sets the padding in RenderThemeWin

This looks fine to me, but Hyatt or Adele should offer an opinion before we accept this.
Comment 3 Dave Hyatt 2008-12-09 10:30:13 PST
Comment on attachment 25799 [details]
Sets the padding in RenderThemeWin

I would query the button style for appearance:none, and if it is none, don't apply this internal padding.  Once you're not a "Windows" button, I don't think the theme should have any influence any more.
Comment 4 Ojan Vafai 2008-12-09 12:09:27 PST
Created attachment 25892 [details]
Sets padding only if style->appearance() != NoControlPart
Comment 5 Dave Hyatt 2008-12-09 13:39:11 PST
Comment on attachment 25892 [details]
Sets padding only if style->appearance() != NoControlPart

r=me
Comment 6 Darin Fisher (:fishd, Google) 2008-12-09 13:46:26 PST
http://trac.webkit.org/changeset/39144