Bug 103453

Summary: LayoutTests/fast/table/css-table-max-height.html failing on Qt Linux
Product: WebKit Reporter: Pravin D <pravind.2k4>
Component: TablesAssignee: Pravin D <pravind.2k4>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, hausmann, jchaffraix, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 87008    
Attachments:
Description Flags
Patch
none
Patch none

Pravin D
Reported 2012-11-27 13:34:33 PST
The test case LayoutTests/fast/table/css-table-max-height.html is failing on Qt linux. Diff http://build.webkit.org/results/Qt%20Linux%20Release/r135895%20%2854971%29/fast/table/css-table-max-height-pretty-diff.html The test passes on other platforms.
Attachments
Patch (2.04 KB, patch)
2012-11-28 00:59 PST, Pravin D 		no flags
DetailsFormatted DiffDiff
Patch (1.91 KB, patch)
2012-11-28 11:12 PST, Pravin D 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Julien Chaffraix
Comment 1 2012-11-27 14:00:34 PST
Test disabled in http://trac.webkit.org/changeset/135917 Failures: This sub-test checks that when content height is greater than max-height, content height is applied to the table with auto layout. FILLER TEXT TO INCREASE CONTENT HEIGHT. FAIL: Expected 192 for height, but got 204. <div class="container"> <div class="child" style="max-height:100px;" data-expected-height="192"> This sub-test checks that when content height is greater than max-height, content height is applied to the table with auto layout. <br><br>FILLER TEXT TO INCREASE CONTENT HEIGHT. </div> </div> [snip] This sub-test checks that when content height is greater than max-height, content height is applied to a table with fixed layout. FILLER TEXT TO INCREASE CONTENT HEIGHT. FAIL: Expected 192 for height, but got 204. <div class="container"> <div class="child fixed-table" style="max-height:100px;" data-expected-height="192"> This sub-test checks that when content height is greater than max-height, content height is applied to a table with fixed layout. <br><br>FILLER TEXT TO INCREASE CONTENT HEIGHT. </div> </div> This test checks that when height of a fixed table is auto, there is are no asserts. No crash means the test passed. PASS
Pravin D
Comment 2 2012-11-28 00:59:02 PST
Created attachment 176420 [details] Patch
Pravin D
Comment 3 2012-11-28 01:05:24 PST
Patch https://bugs.webkit.org/attachment.cgi?id=176420 The default line-height used by Qt-linux is different from the other platforms. This was causing the height of the sub-test cases to be different from expected value. Have also fixed the font-size value, to be on a safer side. @Julien: thanks for the hunch :)
Simon Hausmann
Comment 4 2012-11-28 01:52:04 PST
Julien, can you review this patch? It looks sane to me, but I don't feel comfortable messing with the table tests that I don't know at all :) Thanks :)
Julien Chaffraix
Comment 5 2012-11-28 10:27:28 PST
Comment on attachment 176420 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176420&action=review > LayoutTests/fast/table/css-table-max-height.html:11 > font-family:ahem; > + line-height:1; > + font-size:16px; You can use the shorthand here: font: 16px/1 Ahem; It's more compact thus better.
Pravin D
Comment 6 2012-11-28 11:12:12 PST
Created attachment 176527 [details] Patch
Pravin D
Comment 7 2012-11-28 11:13:18 PST
(In reply to comment #5) > (From update of attachment 176420 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176420&action=review > > > LayoutTests/fast/table/css-table-max-height.html:11 > > font-family:ahem; > > + line-height:1; > > + font-size:16px; > > You can use the shorthand here: > > font: 16px/1 Ahem; > > It's more compact thus better. > Done..
WebKit Review Bot
Comment 8 2012-11-28 12:14:46 PST
Comment on attachment 176527 [details] Patch Clearing flags on attachment: 176527 Committed r136041: <http://trac.webkit.org/changeset/136041>
WebKit Review Bot
Comment 9 2012-11-28 12:14:49 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 10 2012-11-28 15:24:50 PST
(In reply to comment #3) > The default line-height used by Qt-linux is different from the other platforms. Really? Why?
Julien Chaffraix
Comment 11 2012-12-11 12:15:51 PST
(In reply to comment #10) > (In reply to comment #3) > > The default line-height used by Qt-linux is different from the other platforms. > > Really? Why? It seems to be a matter of used font which impacts the height of a line. I remember hitting this issue myself and bug 74219 confirms it too. Regardless of what I just said, it's always good to limit the test variation on the font / line-height selection as it reduces platform-specificity.
Note You need to log in before you can comment on or make changes to this bug.