Bug 91952

Created attachment 153711 [details] Patch

Attachment 153711 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/forms/button-width-height..." exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

kyungtae, bug title and changelog are wrong.

Created attachment 153712 [details] Patch

Created attachment 153715 [details] Patch

Comment on attachment 153715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153715&action=review Thanks for the patch. I recommend doing some research before working on a patch like this -- checking other ports' implementations of RenderTheme::adjustButtonStyle(), I came across GTK+'s, added in r69436. That revision, on its turn, points to an existing layout test that has a similar purpose to the one you have added, and which we are already skipping because it currently fails. And checking LayoutTests/platform/efl/TestExpectations, it is possible to see that the issue is being tracked in bug 85494. I thus suggest closing this bug as a duplicate of bug 85494, checking if the existing test is enough to cover this change and sending a patch there. > Source/WebCore/ChangeLog:3 > + [EFL]Not to adjust height of buttons Nit: we normally add a space between '[Category]' and the rest of the description. Plus "Not to adjust" doesn't look right, "Do not adjust" sounds better. > Source/WebCore/ChangeLog:8 > + The style for a width and a height should be applyed also for a button element. s/applyed/applied/. > Source/WebCore/ChangeLog:10 > + The code should be removed because on other ports, the force height setting code for Buttons not exists. Nitpicking a little on this reasoning, just because other ports don't do that isn't a good explanation; you should rather explain why it does not make sense to use Auto here.

Thank you for the comment. I agree with you. The new layout test is not needed because fast/css/button-height.html already exists. But that test case is still failed after this patch because the difference between <button> and <input type='button'>. The border is different and I think I need to analyze it and modify it, too. RenderButton {BUTTON} size 65x40 [bgcolor=#C0C0C0] [border: (2px outset #C0C0C0)] RenderButton {INPUT} size 61x36 [bgcolor=#C0C0C0]

Adding under meta bug 85877 that's used to track this kind of problem.

*** This bug has been marked as a duplicate of bug 85494 ***