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+

Ojan Vafai
Reported 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.
Attachments
Sets the padding in RenderThemeWin (3.99 KB, patch)
2008-12-05 16:18 PST, Ojan Vafai 		hyatt: review-
DetailsFormatted DiffDiff
Sets padding only if style->appearance() != NoControlPart (5.36 KB, patch)
2008-12-09 12:09 PST, Ojan Vafai 		hyatt: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2008-12-05 16:18:49 PST
Created attachment 25799 [details] Sets the padding in RenderThemeWin
Eric Seidel (no email)
Comment 2 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.
Dave Hyatt
Comment 3 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.
Ojan Vafai
Comment 4 2008-12-09 12:09:27 PST
Created attachment 25892 [details] Sets padding only if style->appearance() != NoControlPart
Dave Hyatt
Comment 5 2008-12-09 13:39:11 PST
Comment on attachment 25892 [details] Sets padding only if style->appearance() != NoControlPart r=me
Darin Fisher (:fishd, Google)
Comment 6 2008-12-09 13:46:26 PST
http://trac.webkit.org/changeset/39144
Note You need to log in before you can comment on or make changes to this bug.