Bug 25016 - max-width and max-height are not overriding the width and height properties when 'display' set to 'table'
Summary: max-width and max-height are not overriding the width and height properties w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on: 98455 98633
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-02 17:59 PDT by jasneet
Modified: 2012-12-18 11:10 PST (History)
12 users (show)

See Also:


Attachments
testcase (1.62 KB, text/html)
2009-04-02 18:00 PDT, jasneet
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description jasneet 2009-04-02 17:59:52 PDT
I Steps:
1. Go to attached testcase

II Issue: 
width and height are overriding max-width and max-height (max-width and max-height must override width and height)

III Other Browsers:
FF3: OK
IE7: OK 

IV Nightly tested: 42138

Bug in Chromium : http://code.google.com/p/chromium/issues/detail?id=9550
Comment 1 jasneet 2009-04-02 18:00:37 PDT
Created attachment 29213 [details]
testcase
Comment 2 Daniel Katz 2009-04-02 22:38:02 PDT
There is a minor mistake in 'Other browsers' section: IE7 doesn't support 'display:table' at all, so originally I've tested the test case on IE8.

IE8: OK
Comment 3 Eric Seidel (no email) 2009-06-15 14:48:01 PDT
Hyatt?

I have confirmed that our behavior does not match FF3.
Comment 4 Eric Seidel (no email) 2009-06-15 15:10:14 PDT
Hyatt says that tables have had this bug forever.  In general our table support needs improving.  Perhaps a member of the Tokyo team would be up to the task.
Comment 5 Julius Stroffek 2011-08-21 13:43:52 PDT
Any chance that somebody will look into this?
Are there any workarounds?
Comment 6 Stephen Cantini 2012-05-06 10:14:47 PDT
+1 Some time has passed... any news? The only workaround I have found is wrapping the table inside a div with "max-width" set...a css-only solution would be much appreciated.
Comment 7 Eric Seidel (no email) 2012-05-06 10:22:23 PDT
The bug is presumably in RenderBox::computeLogicalHeight:
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderBox.cpp#L2025
Comment 8 Stephen Cantini 2012-05-06 10:49:56 PDT
DISCLAIMER: I've never looked at the webkit code before.

Could it be in RenderBox::computeLogicalWidthInRegion() for the ignored "max-width" part? If this is the case, maybe it never reaches this if statement http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderBox.cpp#L1765
Comment 9 Stephen Cantini 2012-09-28 08:27:04 PDT
Any news from the tokyo team or anybody else on this issue? Is there anything else I can do to help solve it? Someone to contact directly?
Comment 10 Tony Chang 2012-09-28 12:05:24 PDT
The bug is probably in RenderTable::updateLogicalWidth.  You can see that it checks against min-width, but there's no code for max-width.
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderTable.cpp?rev=129929#L258

I'm not sure how table height gets computed, but if you look in RenderBox::computeLogicalHeight, it only calculates margins.  I suspect the height is updated by it's children or something.
Comment 11 Julien Chaffraix 2012-09-28 13:01:03 PDT
> I'm not sure how table height gets computed, but if you look in RenderBox::computeLogicalHeight, it only calculates margins.  I suspect the height is updated by it's children or something.

The logical height computation is in RenderTable::layout(). We need to check for the min / max logical height and update |computedLogicalHeight| accordingly.

Note that the effect of min / max width / height on tables is undefined in CSS 2.1 (http://www.w3.org/TR/CSS21/visudet.html#propdef-max-width) so we will need to evaluate what other browsers are doing in some corner cases (e.g. should we shrink our content if max-height is defined? (we never shrink our height currently as it would violate CSS 2.1 tables: we could make a cell smaller than the minimum height for its content))
Comment 12 Julien Chaffraix 2012-12-18 11:10:05 PST
After bug 98455 and bug 98633, this is fixed.