Bug 35231 - [Chromium] Paddings of Mac chromium's buttons don't agree with Firefox
Summary: [Chromium] Paddings of Mac chromium's buttons don't agree with Firefox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-22 00:47 PST by Shinichiro Hamaji
Modified: 2010-02-23 22:16 PST (History)
4 users (show)

See Also:


Attachments
Patch v1 (6.82 KB, patch)
2010-02-22 00:50 PST, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v2 (18.73 KB, patch)
2010-02-22 01:01 PST, Shinichiro Hamaji
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Shinichiro Hamaji 2010-02-22 00:50:52 PST
Created attachment 49192 [details]
Patch v1
Comment 2 WebKit Review Bot 2010-02-22 00:53:58 PST
Attachment 49192 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/rendering/RenderThemeChromiumSkia.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Shinichiro Hamaji 2010-02-22 01:00:08 PST
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/rendering/RenderThemeChromiumSkia.h:35:  Code inside a namespace should
> not be indented.  [whitespace/indent] [4]
> Total errors found: 1 in 6 files

This is known as Bug 33925, but now I decided to fix this style violation in this chance. Please review Patch v1 to check the logical change of this file.
Comment 4 Shinichiro Hamaji 2010-02-22 01:01:15 PST
Created attachment 49193 [details]
Patch v2
Comment 5 Kent Tamura 2010-02-22 01:14:31 PST
This behavior matches to Safari/Mac.  We should change the Safari/Mac behavior too if you want to change this.
Comment 6 Shinichiro Hamaji 2010-02-22 01:22:44 PST
(In reply to comment #5)
> This behavior matches to Safari/Mac.  We should change the Safari/Mac behavior
> too if you want to change this.

Thanks for your comment. Yes, Mac Safari and Win Safari disagree with Firefox, Win Chrome, and Linux Chrome behavior. I don't know the discussion about Bug 24048, but I guessed the decision of Chromium team was different from Safari team. Ojan, could you check if my guess is correct?
Comment 7 Ojan Vafai 2010-02-22 14:39:04 PST
Ideally, form controls for Safari and Chrome will have the same metrics per-platform.

I've actually been meaning to file a bug that we *remove* the 3px padding on Windows/Linux. It causes more compatibility problems than it fixes. Before removing the 3px padding, I wanted to make a screenshot of what it would look like with and without to run it by Chrome's UI leads to make sure they're OK with the change.

Matching Firefox is not in and of itself a goal. Chrome made that change a long time ago (when there was only a Windows Chrome) in an effort to maximize web compatibility, but to date, we've only seen cases where it has hurt compatibility.
Comment 8 Eric Seidel (no email) 2010-02-22 14:43:31 PST
Comment on attachment 49193 [details]
Patch v2

r- based on Ojan's comments.
Comment 9 Shinichiro Hamaji 2010-02-23 22:16:07 PST
Thanks Ojan for the description!

> I've actually been meaning to file a bug that we *remove* the 3px padding on
> Windows/Linux. It causes more compatibility problems than it fixes. Before
> removing the 3px padding, I wanted to make a screenshot of what it would look
> like with and without to run it by Chrome's UI leads to make sure they're OK
> with the change.
> 
> Matching Firefox is not in and of itself a goal. Chrome made that change a long
> time ago (when there was only a Windows Chrome) in an effort to maximize web
> compatibility, but to date, we've only seen cases where it has hurt
> compatibility.

If it's OK to remove the extra paddings, I agree it's the better solution. So, could you have a discussion with UI guys and take over http://crbug.com/1437 ? Or, if you are busy, I'll make screenshots and send them to UI leads. In this case, could you tell me who I should contact and what kind of screenshots I should send, please?

Thanks again!