RESOLVED FIXED 24048
extra windows button padding doesn't apply when there's no appearance
https://bugs.webkit.org/show_bug.cgi?id=24048
Summary extra windows button padding doesn't apply when there's no appearance
Ojan Vafai
Reported 2009-02-19 17:43:19 PST
To match FF, we need to add non-overridable 3px padding-left/right and 1px padding-top/bottom. Currently we do so in RenderThemeWin via adjustButtonInnerStyle only if the button has an appearance. We should do this whether the button has an appearance or not. As per hyatt's suggestion, will replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom and get rid of the hasAppearance check.
Attachments
Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. (10.13 KB, patch)
2009-02-23 14:52 PST, Ojan Vafai
no flags
Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. (10.01 KB, patch)
2009-02-26 12:51 PST, Ojan Vafai
no flags
Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. (10.17 KB, patch)
2009-02-26 12:53 PST, Ojan Vafai
adele: review+
Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. (10.17 KB, patch)
2009-02-26 13:46 PST, Ojan Vafai
no flags
Fixes missing "}" that snuck in on my last sync. (10.17 KB, patch)
2009-02-26 13:48 PST, Ojan Vafai
no flags
Ojan Vafai
Comment 1 2009-02-23 14:52:31 PST
Created attachment 27894 [details] Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. WebCore/ChangeLog | 34 ++++++++++++++++++++++++++ WebCore/rendering/RenderButton.cpp | 7 ++++- WebCore/rendering/RenderTheme.cpp | 4 --- WebCore/rendering/RenderTheme.h | 8 ++++- WebCore/rendering/RenderThemeChromiumGtk.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeChromiumGtk.h | 5 +++- WebCore/rendering/RenderThemeChromiumWin.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeChromiumWin.h | 5 +++- WebCore/rendering/RenderThemeWin.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeWin.h | 5 +++- 10 files changed, 108 insertions(+), 29 deletions(-)
Ojan Vafai
Comment 2 2009-02-26 12:51:11 PST
Created attachment 28032 [details] Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. WebCore/ChangeLog | 34 ++++++++++++++++++++++++++ WebCore/rendering/RenderButton.cpp | 7 ++++- WebCore/rendering/RenderTheme.cpp | 4 --- WebCore/rendering/RenderTheme.h | 8 ++++- WebCore/rendering/RenderThemeChromiumGtk.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeChromiumGtk.h | 5 +++- WebCore/rendering/RenderThemeChromiumWin.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeChromiumWin.h | 5 +++- WebCore/rendering/RenderThemeWin.cpp | 23 +++++++++++++---- WebCore/rendering/RenderThemeWin.h | 5 +++- 10 files changed, 108 insertions(+), 29 deletions(-)
Ojan Vafai
Comment 3 2009-02-26 12:53:44 PST
Created attachment 28033 [details] Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. WebCore/ChangeLog | 36 ++++++++++++++++++++++++++ WebCore/rendering/RenderButton.cpp | 7 +++- WebCore/rendering/RenderTheme.cpp | 4 --- WebCore/rendering/RenderTheme.h | 8 ++++- WebCore/rendering/RenderThemeChromiumGtk.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumGtk.h | 5 +++- WebCore/rendering/RenderThemeChromiumWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumWin.h | 5 +++- WebCore/rendering/RenderThemeWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeWin.h | 5 +++- 10 files changed, 110 insertions(+), 29 deletions(-)
Ojan Vafai
Comment 4 2009-02-26 13:46:42 PST
Created attachment 28035 [details] Replace adjustButtonInnerStyle with buttonInternalPaddingLeft/Right/Top/Bottom. WebCore/ChangeLog | 36 ++++++++++++++++++++++++++ WebCore/rendering/RenderButton.cpp | 7 +++- WebCore/rendering/RenderTheme.cpp | 4 --- WebCore/rendering/RenderTheme.h | 8 ++++- WebCore/rendering/RenderThemeChromiumGtk.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumGtk.h | 5 +++- WebCore/rendering/RenderThemeChromiumWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumWin.h | 5 +++- WebCore/rendering/RenderThemeWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeWin.h | 5 +++- 10 files changed, 110 insertions(+), 29 deletions(-)
Ojan Vafai
Comment 5 2009-02-26 13:48:22 PST
Created attachment 28036 [details] Fixes missing "}" that snuck in on my last sync. WebCore/ChangeLog | 36 ++++++++++++++++++++++++++ WebCore/rendering/RenderButton.cpp | 7 +++- WebCore/rendering/RenderTheme.cpp | 4 --- WebCore/rendering/RenderTheme.h | 8 ++++- WebCore/rendering/RenderThemeChromiumGtk.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumGtk.h | 5 +++- WebCore/rendering/RenderThemeChromiumWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeChromiumWin.h | 5 +++- WebCore/rendering/RenderThemeWin.cpp | 23 ++++++++++++---- WebCore/rendering/RenderThemeWin.h | 5 +++- 10 files changed, 110 insertions(+), 29 deletions(-)
David Levin
Comment 6 2009-03-02 11:17:45 PST
Committed as r41362.
Jouni Koivuviita
Comment 7 2009-07-02 06:43:46 PDT
Hi, Is there still a way to prevent these additional paddings? If you're trying to mimic Firefox's behaviour, you should provide a way to remove the additional paddings as well. In Firefox they can be removed with the following CSS (the double colon is not a typo): button::-moz-focus-inner { border: none; padding: 0; } And why does this concern only the Windows version, and not the OS X version as well? Firefox adds the extra paddings on the OS X version as well.
Jouni Koivuviita
Comment 8 2009-07-22 01:37:50 PDT
Any other comments? Is this a dead issue? Should I open a new ticket?
Shinichiro Hamaji
Comment 9 2010-02-18 05:18:28 PST
Belated response... > Is there still a way to prevent these additional paddings? I think there are no way, unfortunately. > If you're trying to mimic Firefox's behaviour, you should provide a way to > remove the additional paddings as well. In Firefox they can be removed with the > following CSS (the double colon is not a typo): > > button::-moz-focus-inner { > border: none; > padding: 0; > } I agree with you. I've just opened another bug and sent a patch to add -webkit-focus-inner for buttons. Please check Bug 35090 for detail. > And why does this concern only the Windows version, and not the OS X version as > well? Firefox adds the extra paddings on the OS X version as well. Agreed. This should be much easier to fix. I'll open another bug and try fixing this issue, too.
Note You need to log in before you can comment on or make changes to this bug.