REGRESSION (r176751): line-height ignored in <button> elements
Created attachment 251705 [details] Patch
<rdar://problem/19297258>
Comment on attachment 251705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251705&action=review > Source/WebCore/ChangeLog:9 > +2015-04-26 Darin Adler <darin@apple.com> > + > + REGRESSION (r176751): line-height ignored in <button> elements > + https://bugs.webkit.org/show_bug.cgi?id=144234 > + > + Reviewed by NOBODY (OOPS!). > + > + Test: fast/forms/button-line-height.html > + It would also be good to explain what the problem was.
Comment on attachment 251705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251705&action=review > LayoutTests/fast/forms/button-line-height.html:2 > +<button style="height:24px; margin:0">Button with height</button> Does this line test anything? The ref is identical. > LayoutTests/fast/forms/button-line-height.html:4 > +<button style="height:124px; margin:0">Button with height</button> This too.
(In reply to comment #4) > Comment on attachment 251705 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251705&action=review > > > LayoutTests/fast/forms/button-line-height.html:2 > > +<button style="height:24px; margin:0">Button with height</button> > > Does this line test anything? The ref is identical. It’s just there so when you look at the test buttons in the browser you can compare one with the other right next to it. It’s not really part of the reftest, but I like having it there. Probably should have more things in the test explaining what is there and why. But I’ll just leave it as is for now. > > LayoutTests/fast/forms/button-line-height.html:4 > > +<button style="height:124px; margin:0">Button with height</button> > > This too. Ditto.
http://trac.webkit.org/changeset/183366 and http://trac.webkit.org/changeset/183367
This reference test fails on all of the non-Mac/non-iOS ports. I don't understand why there would be a difference in behavior here, unless Windows (and GTK/EFL) have different default button heights than Mac.
Oops. Maybe just a difference in the exact number of pixels added by padding. I’ll be happy to improve the tests at some point. Feel free to skip them for now.
The test is still broken, who can take care of it? https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fforms%2Fbutton-line-height.html
I’d be happy to do something about the test, but I don’t understand how to read the flakiness dashboard output at that URL. Does it mean the test is failing on all platforms? Or that it’s “flaky” on Mac and failing on others? I am happy to replace the test with a different one, but I don’t understand the current status well enough to know what problem I am tackling. The test relies on a specific relationship between the inner and outer dimensions of buttons that I now believe is platform specific. But there may be other problems with the test, too, and I would like to know that before I try to fix it.
Nikita skipped the test in bug 183862. Before the skipping, the flakiness dashboard showed blue for every revision on the below bots, meaning that they failed pixel tests every time (there is a legend for the colors): - Apple Win 7 Debug (Tests) - Apple Win 7 Release (Tests) - EFL Linux 64-bit Release WK2 - GTK Linux 64-bit Debug (Tests) - GTK Linux 64-bit Release (Tests) This is all the test queues except for Mac ones. If you click on one of the blue rectangles, there is a "build log" link, pointing to a buildbot results page. For example, this is how this test fails on Windows: https://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/r183842%20(66058)/fast/forms/button-line-height-diffs.html