Last Comment Bug 77019 - <tr style="visibility: collapse;"> looks like hidden and collapsed
: <tr style="visibility: collapse;"> looks like hidden and collapsed
Status: RESOLVED FIXED
[awd:tbl]
: css2, regression, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: All All
: P3 normal with 2 votes (vote)
: Future
Assigned To: Bernd
: Madhur Bhatia
Mentors:
: 101790 205459 (view as bug list)
Depends on:
Blocks: 76497 149101
  Show dependency treegraph
 
Reported: 2001-04-21 03:58 PDT by Martin Honnen
Modified: 2004-05-02 05:56 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case(bug demo): one row is shown empty but should be collapsed instead (846 bytes, text/html)
2001-04-21 03:59 PDT, Martin Honnen
no flags Details
simple testcase. (590 bytes, text/html)
2002-06-04 09:20 PDT, Mat
no flags Details
patch (2.29 KB, patch)
2002-11-18 10:16 PST, Bernd
no flags Details | Diff | Review
patch (713 bytes, patch)
2003-01-12 08:07 PST, Bernd
no flags Details | Diff | Review
revised patch (12.00 KB, patch)
2003-06-05 12:23 PDT, Bernd
bzbarsky: review+
dbaron: superreview+
Details | Diff | Review
Yet another version... Very similar to the one described before. It works fine but the table keeps growing. If you look at the code after it as growned, the code looks just fine! (1.09 KB, text/html)
2003-10-23 06:12 PDT, JotaPe
no flags Details
Interactive demo page (4.85 KB, text/html)
2004-04-29 14:49 PDT, Gérard Talbot
no flags Details

Description Martin Honnen 2001-04-21 03:58:17 PDT
CSS2 allows for
  visibility: collapse;
on tr elements, however M0.8.1 on Windows 95 doesn't collapse the row but simply 
 hides its content.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
                        "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<title>
row collapsing
</title>
</head>
<body>
<input type="button" value="collapse/expand"
       onclick="if (document.getElementById('aTable').rows[3].style.visibility 
== 'collapse')
                  document.getElementById('aTable').rows[3].style.visibility = 
'';
                else
                  document.getElementById('aTable').rows[3].style.visibility = 
'collapse';"
>
<table id="aTable" border="1">
<script>
for (var i = 0; i < 10; i++)
  if (i == 3)
    document.write('<tr style="visibility: collapse;"><td>' + i + 
'<\/td><td>Kibology<\/td><\/tr>');
  else
    document.write('<tr><td>' + i + '<\/td><td>Kibology<\/td><\/tr>');
</script>
</table>
</body>
</html>
Comment 1 Martin Honnen 2001-04-21 03:59:46 PDT
Created attachment 31722 [details]
test case(bug demo): one row is shown empty but should be collapsed instead
Comment 2 Pierre Saslawsky 2001-04-24 23:43:43 PDT
Reassigned to HTMLTables.
Comment 3 Jeffrey Baker 2001-06-01 13:19:47 PDT
Hey this is a big time problem.  imho must-fix for 1.0.  Another testcase for
this bug is http://bugzilla.mozilla.org/showattachment.cgi?attach_id=16059.
Comment 4 Jeffrey Baker 2001-06-01 13:21:12 PDT
I was going to mention that this used to work, so it should be considered a
regression.  Some people have speculated that paint or reflow suppression may
have broken this.
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2001-09-26 12:35:57 PDT
*** Bug 101790 has been marked as a duplicate of this bug. ***
Comment 6 karnaze (gone) 2002-01-07 09:38:44 PST
Temporarily moving to future until a milestone can be assigned. 
Comment 7 Don Eitner 2002-05-11 02:09:00 PDT
This issue is similar to bug 76497 (which is about collapsing a table's column
rather than row).  I have found some cases where both rows and columns do and do
not behave correctly in Mozilla 1.0.0+ nightly builds (I am using build
2002051008 for OS/2).

Note the test in the Debug menu, Viewer Demos sub-menu, #4 Simple Tables.  The
first two examples of row and column collapsing can be extracted (copy and paste
from the page source to a new page) and they display collapsed rows and
collapsed columns as expected (see http://syntheticdimension.net/test4.html
which has been validated as HTML 4.01 by the w3c).

But then if we remove the third and fourth tables from that (simply deleting
those tables from the HTML code and leaving absolutely everything else in the
file intact) the first example of collapsed rows and collapsed columns fails
(see http://syntheticdimension.net/test4a.html which has also been validated as
HTML 4.01 by the w3c).

Both pages are valid HTML and use exactly the same code for the first and second
tables.  The only difference is that the second test page above has had the
third and fourth tables deleted.  This simple act causes table 2 to display
incorrectly in test4a.html where it worked correctly in test4.html.  Same code
but different display even in the same browser version.  And this was the code
taken from the Mozilla debug tests which are included with the browser binaries.
Comment 8 Mat 2002-06-04 09:20:17 PDT
Created attachment 86233 [details]
simple testcase.

I just ran accross this bug today, here's my testcase :)
Comment 9 Mat 2002-06-04 09:22:53 PDT
see http://www.w3.org/TR/REC-CSS2/tables.html#dynamic-effects for a description
of what should happen.
Comment 10 Bernd 2002-11-17 10:34:27 PST
This is an incremental reflow bug, the wrong code is at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#2209

2209   if (eReflowReason_Resize == aReflowState.reason) {
2210     if (!DidResizeReflow()) {
2211       // XXX we need to do this in other cases as well, but it needs to be
made more incremental
2212       aDoCollapse = PR_TRUE;
2213       SetResizeReflow(PR_TRUE);
2214     }
2215   }

if eReflowReason_Incremental == aReflowState.reason aDoCollapse is not set and
as a consequence the code is not executed.
Comment 11 Bernd 2002-11-18 10:16:21 PST
Created attachment 106678 [details] [diff] [review]
patch

Oh Noooooo, we had the collapse code working in the tree for years and it was
simply optimized away. Hmm may be optimized is not the correct word for it.
Comment 12 Bernd 2003-01-12 08:07:09 PST
Created attachment 111336 [details] [diff] [review]
patch

Chris, I cant imagine how this can be further optimized without having reflow
bugs. This may cause a performance penalty. I hope the penalty will be balanced
with the patch in bug 85755.
Comment 13 karnaze (gone) 2003-01-17 09:38:02 PST
I would consider two options (1) avoid calling AdjustForCollapsingRows/Cols if
there is nothing to collapse or (2) make your patch more efficient. 

(1) may involve setting a bit on nsTableFrame indicating that there are
rows/cols to collapse and then making sure the bit is kept accurate. This could
be a bit of a pain.

(2) make AdjustForCollapsingRows and CollapseRowGroupIfNecessary more efficient.
They process unaffected rows/cols unnecessarily. For example, 

      } else { // row is not collapsed but needs to be adjusted by those that are
        rowRect.y -= aYGroupOffset;
        rowFrame->SetRect(aPresContext, rowRect);
      }

could be changed to 

      } else if (aYGroupOffset != 0) { // row is not collapsed but needs to be
adjusted by those that are
        rowRect.y -= aYGroupOffset;
        rowFrame->SetRect(aPresContext, rowRect);
      }

(2) may be sufficient if this and similar optimizations are made.

The following should also be dealt with at some point and (1) might be a lot
faster if there are a lot of rows in the table.

    // XXX Is this needed?
#if 0
    AdjustForCollapsingRows(aPresContext, aDesiredSize.height);
    AdjustForCollapsingCols(aPresContext, aDesiredSize.width);
#endif

Comment 14 karnaze (gone) 2003-03-31 11:30:11 PST
mass reassign to default owner
Comment 15 Bernd 2003-05-11 10:45:23 PDT
Comment on attachment 111336 [details] [diff] [review]
patch

I will follow path 1) as it promises more pain. I will not further optimize the
code. My idea is:
add a special bit to the mbits indicating that we have collapsed frames.
add in every frame that can be collapsed a style query for the visibility if
the frame is collapsed set the bit.
If the bit is set  in the table frame compute the collapsing for rows/cols but
reset the bit before. If during the computation a frame still needs the
collapsing set the bit again.
Comment 16 Bernd 2003-05-13 04:22:06 PDT
*** Bug 205459 has been marked as a duplicate of this bug. ***
Comment 17 Bernd 2003-06-05 12:23:44 PDT
Created attachment 125017 [details] [diff] [review]
revised patch
Comment 18 JotaPe 2003-10-23 06:12:59 PDT
Created attachment 133930 [details]
Yet another version...
Very similar to the one described before.
It works fine but the table keeps growing. If you look at the code after it as growned, the code looks just fine!

Using style="display:none|block" the table keeps growing.
Unlike other browser( works well in IE) the table grows when you hide rows.
Comment 19 Boris Zbarsky [:bz] (Out June 25-July 6) 2003-11-16 10:18:53 PST
The growing thing is a different bug, and the correct display value for a table
row is "table-row" if you want it to render like a table row.
Comment 20 Boris Zbarsky [:bz] (Out June 25-July 6) 2003-11-16 10:25:31 PST
Comment on attachment 125017 [details] [diff] [review]
revised patch

Seems reasonable.  r=bzbarsky.

For future reference, using 'diff -pu8' would have made this patch _much_
easier to review...
Comment 21 Vicente Salvador 2004-03-04 04:10:09 PST
Review exists. Why not commit this patch and let us to test it ???
Comment 22 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-03-31 14:39:05 PST
Comment on attachment 125017 [details] [diff] [review]
revised patch

sr=dbaron, although drop the weird whitespace change in
nsTableColGroupFrame.cpp.  Sorry for the delay.
Comment 23 Bernd 2004-04-28 11:15:27 PDT
fix checked in
Comment 24 Gérard Talbot 2004-04-29 14:49:45 PDT
Created attachment 147353 [details]
Interactive demo page

While using Mozilla 1.8a build 2004042909, I still see problems in this demo
page and I think this bugfile should be reopened.
Comment 25 Gérard Talbot 2004-04-29 14:54:21 PDT
Sorry, I forgot to explicit the problem I was seeing.
In the interactive demo case, click the collapse button and then the visible
button. The row reappears but is hidden: the text does is not visible even
though the row structure is present.
Comment 26 Bernd 2004-04-29 21:17:34 PDT
please open a new bug on this
Comment 27 Bernd 2004-05-01 10:11:29 PDT
I filed bug 242253 on the paint issue when rows become visible again.
Comment 28 Gérard Talbot 2004-05-02 05:56:25 PDT
Sorry Bernd, I wanted to open a bugfile on this but just did not have the time to.

Note You need to log in before you can comment on or make changes to this bug.