Bug 35231 – [Chromium] Paddings of Mac chromium's buttons don't agree with Firefox

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Shinichiro Hamaji]

Bug 24048 made Win/Linux chromium agree with Firefox, but Mac chromium wasn't fixed. Currently, Win/Linux chromium and Firefox have 3 pixels left/right paddings and 1 pixel up/bottom paddings but Mac chromium doesn't have the paddings.

The test case:

http://www.plexode.com/cgi-bin/eval3.py#ht=%3Cp%3E%0AWin%2FLinux%20chrome%3A%2094%3Cbr%3E%0AMac%20chrome%3A%20100%3Cbr%3E%0AFirefox%3A%2094%3Cbr%3E%0A%3Cp%3E%0A%3Cbutton%20style%3D%22width%3A100px%3B%20border%3A%20none%3B%20padding%3A0px%3B%22%3E%0A%20%3Cdiv%20id%3D%22t%22%3Efoo%3C%2Fdiv%3E%0A%3C%2Fbutton%3E%0A&ohh=1&ohj=1&jt=%22width%3D%22%20%2B%20document.getElementById(%22t%22).offsetWidth&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1

[Shinichiro Hamaji]

Created attachment 49192 [details]
Patch v1

[WebKit Review Bot]

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.

[Shinichiro Hamaji]

> 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.

[Shinichiro Hamaji]

Created attachment 49193 [details]
Patch v2

[Kent Tamura]

This behavior matches to Safari/Mac.  We should change the Safari/Mac behavior too if you want to change this.

[Shinichiro Hamaji]

(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?

[Ojan Vafai]

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.

[Eric Seidel (no email)]

Comment on attachment 49193 [details]
Patch v2

r- based on Ojan's comments.

[Shinichiro Hamaji]

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!