Created attachment 123973 [details] Reproduction In the attached reproduction, the div with display: table (a CSS table) should be 400px wide, which is 50% of its 800px wide parent div. In FF and Opera, the CSS table is 400px wide. In WebKit, it is only 200px wide because the CSS table's 200px padding is included in the CSS table's percent width.
Created attachment 123974 [details] Screenshot Adding a screenshot with renderings from WebKit on top, Opera in the middle, and FF on bottom.
Created attachment 171193 [details] Patch
Created attachment 171381 [details] Patch
Comment on attachment 171381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171381&action=review > Source/WebCore/rendering/RenderTable.cpp:321 > + if (isCSSTable && (styleLogicalWidth.isFixed() || styleLogicalWidth.isPercent()) && styleLogicalWidth.isPositive()) { Shouldn't *any* specified width be handled in this way? This will fix the issue for percent but other values still have the issue (e.g. viewport relative length). We should fix all cases at once by using styleLogicalWidth.isSpecified() instead of just patching our way around. > LayoutTests/fast/table/css-table-width-with-border-padding.html:43 > + </div> This should probably be a check-layout.js (the upside is that it would run faster). > LayoutTests/fast/table/css-table-width-with-border-padding.html:46 > +</html> It would be nice to have some checks against box-sizing values as part of this too. That makes sure we don't break it (which I don't think you do).
Created attachment 172220 [details] Patch
(In reply to comment #4) > (From update of attachment 171381 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=171381&action=review > Thanks for the review Julien. Have uploaded another patch with the changes. > > Source/WebCore/rendering/RenderTable.cpp:321 > > + if (isCSSTable && (styleLogicalWidth.isFixed() || styleLogicalWidth.isPercent()) && styleLogicalWidth.isPositive()) { > > Shouldn't *any* specified width be handled in this way? This will fix the issue for percent but other values still have the issue (e.g. viewport relative length). > > We should fix all cases at once by using styleLogicalWidth.isSpecified() instead of just patching our way around. > Yes, the check should indeed be for all specified values. Have made the change. > > LayoutTests/fast/table/css-table-width-with-border-padding.html:43 > > + </div> > > This should probably be a check-layout.js (the upside is that it would run faster). > Done. > > LayoutTests/fast/table/css-table-width-with-border-padding.html:46 > > +</html> > > It would be nice to have some checks against box-sizing values as part of this too. That makes sure we don't break it (which I don't think you do). Done.
Created attachment 172538 [details] Patch
Patch with modified layout test for verifying viewport relative widths as well.
Created attachment 172544 [details] Patch
Comment on attachment 172544 [details] Patch Attachment 172544 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14728860 New failing tests: fast/table/css-table-width-with-border-padding.html
Comment on attachment 172544 [details] Patch Attachment 172544 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14744471 New failing tests: fast/table/css-table-width-with-border-padding.html
Created attachment 172747 [details] Patch
Comment on attachment 172747 [details] Patch Changed the layout test using check-layout.js to a ref test since verifying the behavior with viewport relative width made the test platform specific.
Created attachment 173029 [details] Patch
Comment on attachment 173029 [details] Patch DRT does force set the width of our viewport to 800 across all platforms. Thus reverting the layout test to use check-layout.js again.
Comment on attachment 173029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173029&action=review I don't need to see the updated patch. > Source/WebCore/ChangeLog:12 > + This was already handled for fixed width CSS tables but not for percent > + width tables. The ChangeLog seems stale as you handle any specified length. Also please rename the bug to cover the extended use case (which includes updating the tests). > LayoutTests/fast/table/css-table-width-with-border-padding.html:86 > + <!-- For box-sizing: padding-box (not supported) --> You should probably mention that the results as checking in are wrong: because we don't support box-sizing: padding-box, the results are 100px longer. You could also check in the expected result. I don't feel strongly about the 2 options (which are equivalent). Filing a bug or commenting on the box-sizing: padding-box change is important in both cases.
Updating bug title.
Created attachment 173225 [details] Patch
Comment on attachment 173225 [details] Patch Clearing flags on attachment: 173225 Committed r134265: <http://trac.webkit.org/changeset/134265>
All reviewed patches have been landed. Closing bug.
Hi, I still encounter the exact same issue. Using Safari 6.0.5 (8536.30.1). Maybe something broke this behavior again?