307866 - min-width/min-height/max-width/max-height does not work for table elements

Status: RESOLVED WORKSFORME

(Core :: Layout: Tables, defect)

Description

[Giampaolo Bellavite]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

See the example here:
http://www.bellavite.com/bugzilla/tbody.htm
the <tbody> tag should display scrollbars in the table thanks to the following
style sheet:
<tbody style="max-height:110px;overflow:auto">
in Firefox 1.5 beta1 this seems not working, as happens in Firefox 1.0.6

Reproducible: Always

Steps to Reproduce:
Create a long table with max-height/overflow:auto style sheet properties in the
<tbody> tag. The max height value should be less height than the table height.
Actual Results:  
Scrollbars don't appears

Expected Results:  
Show scrollbars

I've upgraded from firefox 1.0.6 to firefox 1.5 beta

Comment 1

[Giampaolo Bellavite]

Attachment: Example file

Comment 2

[Giampaolo Bellavite]

I found the CSS2 specification doesn't allow the max-height property in table cells:
http://www.w3.org/TR/REC-CSS2/visudet.html#propdef-max-height
So I guess this is not really a bug.

Comment 3

[Erik Fabert]

In the latest CSS 2.1 working draft, max-height is said to apply to "all 
elements but non-replaced inline elements, table columns, and column groups", so 
it should affect TBODY. 

Comment 4

[Adam Guthrie]

INVALID per comments 2 and 3.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

You misread comment 3.

Comment 6

[Mats Palmgren (inactive)]

Attachment: TABLE min-height

Comment 7

[Mats Palmgren (inactive)]

Attachment: TR min-height

Comment 8

[Mats Palmgren (inactive)]

Attachment: TD min-width/height

Comment 9

[Mats Palmgren (inactive)]

Attachment: TBODY min-height

Comment 10

[Mats Palmgren (inactive)]

Attachment: TBODY max-height

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 12

[Mats Palmgren (inactive)]

Comment on attachment 195979 [details] [diff] [review]
Patch rev. 1

I have spawned off max-height/max-width for table cells to bug 308403,
which this patch does not address (other than setting up a correct
reflow state).

Comment 13

[Bernd]

*** Bug 227650 has been marked as a duplicate of this bug. ***

Comment 14

[Bernd]

Mats, sorry for the delay but its high on my priority list now.

+        if (eStyleUnit_Auto == heightUnit) {
+          mComputedHeight = (mComputedMinHeight == 0) ? NS_AUTOHEIGHT :
mComputedMinHeight;
+        } else {

Could you please convince me that this is the right thing to do.

If a auto height table element has a min height it gets a computed height?

Wouldnt that break for standards mode the pct to auto conversion for children of
such a element?

nsStyleCoord nsTableColFrame::GetStyleWidth() const

I have big doubts that the changes are correct, while usually fixed widths are
the upper limits, with the patch a specified min width on a column turns the
column into a column with a fixed width, I would rather expect that a specified
min width influences rather the MEW than changes a col type.




        

Comment 15

[Bernd]

Comment on attachment 195979 [details] [diff] [review]
Patch rev. 1

The changes to BasicTableLayoutStrategy are wrong. The min-width should update
the MIN_CON
http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableColFrame.h#53
the max-width is an additional constraint which should be added between MIN_PRO
and FINAL. The new constraint should take part in the balancing.
My proposal is to split these widths for colspans just by dividing trough the
real cols that it covers.

Comment 16

[u39892]

Out of curiousity... has this bug been forgotten?

Comment 17

[Boris Zbarsky [:bzbarsky]]

No point working on this until reflow branch lands, since that rewrites pretty much all of BasicTableLayoutStrategy.

Comment 18

[Kelly Miyashiro]

2.0 is out, any news on when the reflow branch will land and when this will be fixed?

Comment 19

[Boris Zbarsky [:bzbarsky]]

Kelly, what 2.0 is doing is completely unrelated to the reflow branch.  The current trunk plan is to stabilize the existing cairo and Cocoa widget work, then ship an alpha of Gecko 1.9 sometime in early to mid November, with any luck, then land the reflow branch on the trunk.  After that, this bug should be revisited.

You can keep track of the current state of reflow branch via http://wiki.mozilla.org/Gecko:Reflow_Refactoring#Status and http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=REFLOW_20060830_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=week&mindate=&maxdate=&cvsroot=%2Fcvsroot

Comment 20

[Raj Bhaskar]

Now that the reflow branch has landed, will this bug be revisited?

Comment 21

[Boris Zbarsky [:bzbarsky]]

At some point, sure.  There are 80-some open bugs (including this one) that are flagged as needing someone to look at them after the branch lands, so...  ;)

Comment 22

[Bernd]

Most of the patch is obsolete due to the reflow branch landing.

Comment 24

[James Barrante]

This is still alive.

Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.9b3pre) Gecko/2008010605 Minefield/3.0b3pre

Comment 26

[Oet]

It's weird, but this bug still seems to be present in FF3.5
I still do not see the expected behaviour on my divs containing tables like FF1.x did...
Please address this issue again. It's really annoying..

Comment 27

[Will Pittenger]

For me, I was setting up a table I wanted to center and set the max-width.  It worked--but only if I used inline styles.  If I use a class, "max-width:792px;" is treated as "width:792px;" and the table is left justified.  To be fair, IE appears to be doing the same.

Comment 28

[Boris Zbarsky [:bzbarsky]]

This bug is about table _internal_ elements, not <table>s.  We don't do anything differently for inline style vs class styling, so it sounds like you just had a mistake in your class styling somewhere.  Please file a separate bug if you think there was a Gecko issue on your testcase, and attach your testcase to that bug.

Comment 29

[Oet]

Hi,

Any news on this bug, this is still current en very much annoying. Any idea on when it will be fixed, it seems to take a long time...

Comment 30

[Danilo Câmara]

This was fixed in Firefox 3.6 but the feature was removed in Firefox 4:

"The overflow property no longer applies to table-group elements (<thead>, <tbody>, and <tfoot>)." https://developer.mozilla.org/en/Firefox_4_for_developers#Miscellaneous_CSS.c2.a0changes

I would like to know why this happened.

This is the test case in W3C:
http://www.w3.org/WAI/UA/TS/html401/cp1001/1001-THEAD-TBODY-TFOOT-OVERFLOW.html

This is the equivalent bug report for WebKit:
https://bugs.webkit.org/show_bug.cgi?id=3239

Comment 31

[Danilo Câmara]

> I would like to know why this happened.

Replying to myself, the answer may be here:

"The main reasons were that if and when overflow _is_ specified to work on these elements we don't want to be poisoning the web with a behavior that's likely different from what the spec says, and that maintaining this feature significantly increased the complexity and decreased the robustness of the table code (which is already pretty complex; the additional complexity was just not warranted)."
https://bugzilla.mozilla.org/show_bug.cgi?id=28800

I'm not sure about the "different from what the spec says".

Comment 32

[Boris Zbarsky [:bzbarsky]]

Danilo, comments 30 and 31 have nothing to do with this bug.

Comment 33

[Daniel Trebbien]

http://lists.w3.org/Archives/Public/www-style/2012Jan/0511.html

https://bugs.webkit.org/show_bug.cgi?id=12396

Comment 34

[Chris]

I do not understand why it takes so long to fix "min-height" because the "height" property is interpreted as minimum height on tables anyway (it's like an alias).

Today, only Firefox does not support this property on table, even with IE 11 it works fine. It's surprising to see a basic layout property not working only on firefox and to do a workaround only for this browser.

I thought maybe nobody reported it, but since 2005 this bug is know. I don't know what is happening, maybe this bug should be splitted in 4 differents bugs (one for each properties).

Comment 36

[Jeff Muizelaar [:jrmuizel]]

This page is broken in Firefox because of this: https://developer.chrome.com/devsummit/sessions/

Comment 37

[Dennis Haney]

I just ran into this problem also, pretty annoying to see my site broken in FF and have to spend extra time having to fix it for a specific browser.

Comment 39

[Keul]

There's duplicates of this bug here (or this is a duplicate of one of them :D) : 
bug 720669
bug 975632
bug 308403
bug 434230
(found with https://bugzilla.mozilla.org/buglist.cgi?quicksearch=max-width+table)

Comment 40

[Boris Zbarsky [:bzbarsky]]

That's not a great search, because....

> bug 720669

Is about an element _inside_ a table cell, not a table element.

> bug 975632

Same thing.

> bug 434230

Same thing.

> bug 308403

Is already marked dependent on this bug.

Comment 42

[Brad]

This bug is more than 10 years old. When might we see a fix for it?

Comment 43

[Karl Dubost💡 :karlcow]

Probably a duplicate Web Compatibility issue.
https://webcompat.com/issues/2818

Comment 44

[Jeff Muizelaar [:jrmuizel]]

I just ran into this again on http://flyweb.github.io/

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think one good thing to do is to figure out which of the 36 combinations of:
 - -height vs. -width (with table-layout: auto) vs. -width (with table-layout: fixed, in the first row of course)
 - min- vs. max-
 - on table vs. column vs column-group vs. row vs. row-group vs. cell
do *something* in other browsers, and whether what they do seems interoperable at a very basic level.

It may be worth filing separate bugs on many of these things rather than trying to do them all in one bug.

Comment 46

[Karl Dubost💡 :karlcow]

David,

(In reply to David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) from comment #45)
> I think one good thing to do is to figure out which of the 36 combinations
> of:
> It may be worth filing separate bugs on many of these things rather than
> trying to do them all in one bug.

I created a reduced test case for one of the Web Compat bug when I was trying to understand what was happening. See

https://webcompat.com/issues/2818#issuecomment-231620400
http://codepen.io/anon/pen/JKAajx

Comment 47

[Square Candy Design]

Surprised to see how old this issue is. @karlcow's codepen illustrates the issue perfectly.

FYI, my current hack to make this work is to add any specific height in addition to the min-height declaration.  Doesn't seem like that should be needed, but it works for now.

http://codepen.io/squarecandy/pen/wWLNAV

Comment 48

[joviczarko]

I have noted that this case now corrected in Firefox Developer Edition 50.0a2 (will come to regular Firefox soon I hope), but now it does not override the height if somewhere before min-height, height is declared in percents (%).

Here is the sample
http://codepen.io/zarkojovic/pen/KgzQwV

Comment 49

[mac]

I'm not seeing this as being corrected in Firefox Dev edition 50.0a2.  I've attached a screenshot of it displaying your code-pen.

http://postimg.org/image/jnqvqwu63/

Comment 50

[Karl Dubost💡 :karlcow]

Adding another case 
https://webcompat.com/issues/2832

Comment 51

[Sylvestre Ledru [:Sylvestre]]

Clearly ni on Jet as he no longer works at Mozilla.

Comment 52

[jasper.speicher]

This is not resolved on quantum. see the above-fold element on http://www.openmhealth.org/

Comment 53

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 54

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 55

[stemind]

Compare also the rendering of https://www.blumenmaarsen.ch/index.php?c=moserstrasse&l=e in Firefox with all other major browsers.

Comment 56

[Karl Dubost💡 :karlcow]

min-height issue has been fixed in Bug 1651530 and that solved most of the webcompat issues

Comment 57

[Karl Dubost💡 :karlcow]

Some of the tests here are still failing for other things.

Comment 58

[NSP Login]

redacted - mhoye

Comment 59

[NSP Login]

redacted - mhoye

Comment 60

[NSP Login]

redacted - mhoye

Comment 61

[Scholarships Post]

redacted - mhoye

Comment 62

[Identified Call 05]

redacted - mhoye

Comment 63

[Computer Typing Test]

redacted - mhoye

Comment 64

[Namkeen Distributorship]

redacted - mhoye

Comment 65

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 66

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 6 duplicates, 23 votes, 51 CCs and 7 See Also bugs.
:dholbert, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 67

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 68

[James Graham [:jgraham]]

I can't see any evidence for current webcompat issues here, and honestly I'm not sure what this bug is supposed to cover that isn't covered by https://bugzilla.mozilla.org/show_bug.cgi?id=308403 If nothing, then maybe this should be closed?

Comment 69

[Daniel Holbert [:dholbert]]

(In reply to James Graham [:jgraham] from comment #68)

honestly I'm not sure what this bug is supposed to cover that isn't covered by https://bugzilla.mozilla.org/show_bug.cgi?id=308403

Per comment 12 and the patch attached there, it looks like Mats intended to address min-width/min-height on this bug, and he spun off bug 308403 as a followup to cover max-width/max-height since his patch didn't handle those. That patch from comment 12 never seems to have landed, though.

We also may have fixed this (or most/much of this) at various points over the years, though? It certainly looks like we're honoring min-height on table in the 1st testcase here, and min-width on td in the 3rd testcase, for example. I do see we have bug 1651530 as an explicit dependency that fixed up some of this, too. Per dbaron's comment 45, it's not particularly useful at this point to have this one bug covering a host of different {min/max}-{height/width} use cases, especially now that we've made progress on fixing some of them, and others of them are interoperably ignored in all browser engines.

Looking at the attached testcases, it seems we're almost entirely interoperable with Chrome except for one case in the 3rd testcase where we do honor min-width and they do not; and we're almost entirely interoperable with WebKit except for that same case, plus some cases where WebKit ignores height on tbody.

Specific results on the testcases here:

• TABLE min-height: We match WebKit/Chromium
• TR min-height: We match WebKit/Chromium
• TD min-width/height: We match WebKit/Chromium, except for the last three lines where we render the three lines the same width and Chrome/WebKit do not (we're honoring the min-width:80% on the first 2 and Chrome/WebKit are not; I guess they don't support percent min-width on cells?)
• TBODY min-height: We match WebKit/Chromium in ignoring the min-height on the tbody there. (WebKit renders one table as shorter than Chrome/Firefox, due to them also ignoring height on tbody, it seems.)
• TBODY max-height: We match Chromium/WebKit in ignoring the max-height there. (As in the previous testcase: WebKit renders one table as shorter than Chrome/Firefox, due to them also ignoring height on tbody, it seems.)

So I think we can close this as WFM.