Last Comment Bug 28800 - Remove the ability for rowgroups to scroll (e.g. <tbody style="overflow:auto">)
: Remove the ability for rowgroups to scroll (e.g. <tbody style="overflow:auto">)
Status: RESOLVED FIXED
[awd:tbl] [PATCH]
: dev-doc-complete, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: All All
: P3 normal with 5 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 74974 101157 154301 187701 205308 224078 (view as bug list)
Depends on: 173277 558655
Blocks: 135236 274473 308408 317137 319167 387343 394518 532374 539342
  Show dependency treegraph
 
Reported: 2000-02-22 04:53 PST by Adam
Modified: 2014-03-27 06:52 PDT (History)
27 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
GIF of Table 5 footer problems (18.09 KB, image/gif)
2000-02-22 04:54 PST, Adam
no flags Details
GIF of Table 5 BEFORE tscroll problem (59.04 KB, image/gif)
2000-02-22 04:56 PST, Adam
no flags Details
GIF of Table 5 AFTER tscroll problems (100.96 KB, image/gif)
2000-02-22 04:58 PST, Adam
no flags Details
HTML file demonstrating problem (1.59 KB, text/html)
2000-09-19 20:05 PDT, Adam
no flags Details
patch to allow space for vertical scroll bar (18.55 KB, patch)
2001-05-02 16:59 PDT, karnaze (gone)
no flags Details | Diff | Review
patch to remove scrollable rowgroups (39.71 KB, patch)
2009-06-30 22:43 PDT, Bernd
bzbarsky: review-
bzbarsky: superreview-
Details | Diff | Review
revised patch (59.76 KB, patch)
2009-07-23 23:08 PDT, Bernd
bzbarsky: review+
Details | Diff | Review
hg wdiff -r qparent -r qtip (57.58 KB, patch)
2009-07-27 12:33 PDT, Bernd
no flags Details | Diff | Review
patch after framelist changes (60.05 KB, patch)
2009-08-06 02:14 PDT, Bernd
bzbarsky: review-
Details | Diff | Review
hash table version (59.59 KB, patch)
2009-10-04 11:33 PDT, Bernd
bzbarsky: review+
Details | Diff | Review
patch updated to tip (59.34 KB, patch)
2010-01-13 13:25 PST, Bernd
no flags Details | Diff | Review

Description Adam 2000-02-22 04:53:18 PST
On Mozilla Mac M14 (2000022108), while browsing the viewer demos, I found a
problem in #4. The problems occur with table 5 (table w/ the scrolling tbody).
The footer is displayed twice, once with one word per line and below that as a
full line of text (as shown in Fig. 1). Next prob. has to do with table
formatting - the fourth column text is cut off by the scroll bar (again, as
shown in Fig. 1). Most significant prob. happens when actually scrolling in the
table. The whole table shifts down the page, creating a 6 in. gap between the
bottom of table 4 and table 5's header. Also, the second footer is not displayed
(see Fig. 2 for pre-scroll; see Fig. 3 for post-scroll). Could be a prob. with
the HTML for demo #4 but no time to check so sending to HTML Tables.
Comment 1 Adam 2000-02-22 04:54:29 PST
Created attachment 5581 [details]
GIF of Table 5 footer problems
Comment 2 Adam 2000-02-22 04:56:00 PST
Created attachment 5582 [details]
GIF of Table 5 BEFORE tscroll problem
Comment 3 Adam 2000-02-22 04:58:18 PST
Created attachment 5583 [details]
GIF of Table 5 AFTER tscroll problems
Comment 4 karnaze (gone) 2000-03-06 17:31:10 PST
The 1st problem is bug 26488.
Comment 5 Adam 2000-04-20 15:51:53 PDT
Can I go ahead and write up a new bug(s) that separate the problems displayed 
here (this is a bug I reported a while back)? Since the first problem is already 
covered in bug 26488, I want to cut that out and make a new bug of the remaining 
problem(s) and mark this a dup. If there's no response, I'll just go ahead (but 
not mark this as a dup).

BTW - the problem where scrolling the table would 'jump' the table down the page 
doesn't occur in Viewer.
Comment 6 Adam 2000-05-15 02:46:58 PDT
Actually, I'm not seeing this anymore as of the 05.10.2000 build. Fixed?
Comment 7 Bernd Mielke 2000-06-12 06:18:32 PDT
Adding donttest keyword so this doesn't show up on the bugathon search page as
the URL is already a mozilla testcase.
Comment 8 karnaze (gone) 2000-06-12 07:34:59 PDT
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another 
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Comment 9 Adam 2000-09-19 20:01:42 PDT
Still one problem with this:

Scrolling table 5 shifts the following contents up a pixel or two. It doesn't
seem to be a major problem (it garbles the caption) but, since it moves all
elements of the page that are after it, it might be possible that it would screw
something up on a complex page.

I'm attaching the relevant portion of the HTML file. If you remove the footer
(<tfoot>) from the file, the problem no longer occurs.
Comment 10 Adam 2000-09-19 20:05:21 PDT
Created attachment 15060 [details]
HTML file demonstrating problem
Comment 11 Adam 2000-10-09 20:46:08 PDT
Updating summary to reflect (what I think is) the remaining problem. Removing URL 
as I haven't found a way to reference local files. Spammity, spam, spam, spam.
Comment 12 Bernd 2000-11-18 10:18:44 PST
Adam are you still seeing the problem? WFM Win98 CVS 2000-11-18.
Comment 13 Adam 2000-11-18 17:44:12 PST
Still seeing this in 11/15 Mac build. Only happens the first time, though.
Reloading the document and the problem no longer occurs.
Comment 14 Christine Hoffman 2000-12-12 11:38:36 PST
Changing keyword from 'donttest' to 'testcase' as there is a testcase provided.
Comment 15 Marcia Knous [:marcia - use ni] 2001-03-08 12:17:48 PST
nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 
Comment 16 Amarendra Hanumanula 2001-03-22 13:17:04 PST
QA contact update
Comment 17 karnaze (gone) 2001-04-30 11:09:47 PDT
Changing summary.
Comment 18 karnaze (gone) 2001-05-02 16:58:23 PDT
Moving to m0.9.1
Comment 19 karnaze (gone) 2001-05-02 16:59:26 PDT
Created attachment 32972 [details] [diff] [review]
patch to allow space for vertical scroll bar
Comment 20 karnaze (gone) 2001-05-02 17:05:35 PDT
The only problem I'm seeing on Win2K is the clipped scrolling <tbody>, which the 
patch addresses.
Comment 21 karnaze (gone) 2001-05-15 13:00:14 PDT
Moving to m0.9.2
Comment 22 karnaze (gone) 2001-05-31 13:43:58 PDT
Moving to m0.9.3
Comment 23 karnaze (gone) 2001-07-25 15:05:25 PDT
*** Bug 74974 has been marked as a duplicate of this bug. ***
Comment 24 Blake Ross 2001-07-31 09:36:11 PDT
Missed 0.9.3.
Comment 25 karnaze (gone) 2001-10-04 11:02:12 PDT
reassigning to m0.9.6
Comment 26 karnaze (gone) 2001-11-06 14:26:00 PST
-->m098. The patch is a bit clugy and needs some re-thinking.
Comment 27 karnaze (gone) 2002-01-15 09:19:11 PST
->m099
Comment 28 Bernd 2002-11-02 04:53:16 PST
*** Bug 101157 has been marked as a duplicate of this bug. ***
Comment 29 Bernd 2002-11-02 06:27:19 PST
*** Bug 154301 has been marked as a duplicate of this bug. ***
Comment 30 karnaze (gone) 2003-03-31 11:03:38 PST
mass reassign to default owner
Comment 31 Bernd 2003-05-12 11:43:17 PDT
*** Bug 205308 has been marked as a duplicate of this bug. ***
Comment 32 Hanspeter Niederstrasser 2003-09-25 15:53:12 PDT
This bug was partly fixed by the recent fix to Bug 173277.  While the test case
content is now fully viewable, it requires hoziontal scrolling to see the
content that was previously hidden by the vertical scrollbar.  Not optimal, but
clearly better than obstructed content.
Comment 33 Mats Palmgren (:mats) 2003-10-29 17:59:02 PST
*** Bug 224078 has been marked as a duplicate of this bug. ***
Comment 34 Bernd 2004-02-03 10:14:48 PST
*** Bug 187701 has been marked as a duplicate of this bug. ***
Comment 35 Bernd 2009-06-23 13:10:48 PDT
no other browser that I am aware scrolls the test case here, I think we should just remove the scrolling.
Comment 36 Bernd 2009-06-30 22:43:39 PDT
Created attachment 386214 [details] [diff] [review]
patch to remove scrollable rowgroups
Comment 37 Bernd 2009-06-30 22:50:31 PDT
this is a log from #developers last week.

	<bernd_>	fantasai: is my reading of the css 2.1 spec correct that overflow does not apply to row groups
	
	<fantasai>	bernd_: Seems so, it's not listed under 'overflow' and I don't see any exceptions listed
	
        <fantasai>	bernd_: But the 'height' property does apply
	<fantasai>	bernd_: So I think there must be a mistake in the spec
	<fantasai>	bernd_: unless height is treated as a min-height
	<fantasai>	bernd_: maybe check what other browsers do and/or post a CSS2.1 issue + testcase to www-style?
	<bernd_>	fantasai: this looks so promising like a fast road to kill the scroll frame its to good to be true
	<bernd_>	fantasai: no other browser scrolls row groups thats why I looked up the spec
	<bernd_>	fantasai: and http://www.w3.org/TR/CSS21/tables.html#height-layout treats heights like min-heights
	<fantasai>	bernd_: yeah, but that's for cells and table-rows
	<fantasai>	bernd_: it doesn't say what to do for table-row-groups
	<roc>	bernd_: I posted to www-style ages ago about scrollable rowgroups
	<roc>	no-one was very interested in them
	<roc>	no other browser deveopers, I mean
	<roc>	people have complained about bugs in it, though
	<roc>	they'd complain more if we removed them I guess
	<roc>	probably we should just remove them though
	<bernd_>	roc: I am sitting at a now 2 pages long thing about the border collapse stuff, and it all circles around those scrollable row groups
	<roc>	kill them
	<bernd_>	If we remove them I would just know what to do
	<fantasai>	I don't think we want scrollable row groups really. Doing something intelligent with headers and footers would be good, though
	<roc>	they don't work very well in our implementation anyway, and they're almost impossible to use effectively
	* fantasai	thinks it would be nice if headers got stuck to the top of the viewport while you scroll through the rest of the table
	<bernd_>	we have a bug on this
	<bernd_>	roc: I will prepare a post to mozilla.dev.tech.layout and propose the removal
	<roc>	how about I just preemptively approve it
	<roc>	but I guess you should so we can say we did
	<bernd_>	roc: moa on removal- thats a late birthday present ( I had my birthday last week)
	<roc>	probably should check with dbaron I guess
	<fantasai>	he's in a meeting right now, but I can't see any reason he'd disapprove
	<fantasai>	it's in line with spec and other implementations
	<roc>	yeah
	<roc>	bernd_: just write the patch
	<roc>	thanks man
Comment 38 Bernd 2009-06-30 22:59:54 PDT
Comment on attachment 386214 [details] [diff] [review]
patch to remove scrollable rowgroups

The attached patch does a little bit more than the removal
- it backs out bug 423823 as it does not apply
- it removes GetRowGroup
- it tries to remove undesired references by replacing them with a
  uniform pointer to the row group, so that conversions stop.
- and finally it tries to make one of the previous callers of GetRowGroup sane
  nsTableFrame::ResetRowIndices was just to complicated for me to read, 
  I needed two attempts of 45 min to parse it.
  And then I asked myself which idiot wrote this and cvs blame can be cruel.
Comment 39 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-01 16:19:43 PDT
>+++ b/layout/base/nsCSSFrameConstructor.cpp
>@@ -8144,17 +8141,17 @@ nsCSSFrameConstructor::CreateContinuingT
>       nsTableRowGroupFrame* rowGroupFrame =
>-        nsTableFrame::GetRowGroupFrame(childFrame);
>+        static_cast<nsTableRowGroupFrame*>(childFrame);
>       if (rowGroupFrame) {

That null-check can't fail, right?  Why is it there?

>+++ b/layout/tables/nsTableFrame.cpp
>@@ -545,38 +544,25 @@ void nsTableFrame::ResetRowIndices(nsIFr

The change here is one that changes the algorithm from O(N) in number of rowgroups to one that is O(N*M) where M is number of rowgroups being inserted.  Is that really desirable?

> nsTableFrame::CollectRows(nsIFrame*                   aFrame,
>                           nsTArray<nsTableRowFrame*>& aCollection)

This function is weird.  Can it ever have a non-rowgroup frame passed to it?  If not, how can that have any kids that aren't rows?  It seems like it can be simplified a good bit...

>@@ -1160,24 +1117,22 @@ nsTableFrame::InsertRowGroups(nsIFrame* 
>+          cellMap->InsertGroupCellMap(static_cast<nsTableRowGroupFrame*> (kidFrame), 

No need for the space before '('.  Or you could just use orderedRowGroups[rgIndex], right?

>@@ -1185,30 +1140,29 @@ nsTableFrame::InsertRowGroups(nsIFrame* 
>+            InsertRows(static_cast<nsTableRowGroupFrame*> (kidFrame),

Same here.

>@@ -2088,17 +2041,17 @@ nsTableFrame::PushChildren(const FrameAr
>   PRUint32 childX;
>   nsIFrame* prevSiblingHint = aFrames.SafeElementAt(aPushFrom - 1);
>   for (childX = aPushFrom; childX < aFrames.Length(); ++childX) {
>     nsIFrame* f = aFrames[childX];
>     // Don't push repeatable frames, do push non-rowgroup frames.
>     // XXXbz Need to push the non-rowgroup frames, even though we don't reflow
>     // them, so that we don't lose them.  Of course there shouldn't be any
>     // non-rowgroup frames here...
>-    nsTableRowGroupFrame* rgFrame = GetRowGroupFrame(f);
>+    nsTableRowGroupFrame* rgFrame = static_cast<nsTableRowGroupFrame*>(f);
>     NS_ASSERTION(rgFrame, "Unexpected non-row-group frame");
>     if (!rgFrame || !rgFrame->IsRepeatable()) {

Are we confident enough in that assert to just nix the null-check and XXX comment?

>@@ -2490,17 +2443,18 @@ nsTableFrame::RemoveFrame(nsIAtom*      
>+    nsTableRowGroupFrame* rgFrame =
>+      static_cast<nsTableRowGroupFrame*>(aOldFrame);
>     if (rgFrame) {

And to nix this null-check?

> nsTableFrame::InitChildReflowState(nsHTMLReflowState& aReflowState)
>+    nsTableRowGroupFrame* rgFrame =
>+       static_cast<nsTableRowGroupFrame*>(aReflowState.frame);
>     if (rgFrame) {

This null-check is now pointless.

>@@ -2691,17 +2646,18 @@ nsTableFrame::OrderRowGroups(RowGroupArr
>+    nsTableRowGroupFrame* rowGroup =
>+      static_cast<nsTableRowGroupFrame*>(kidFrame);
>     if (NS_LIKELY(rowGroup)) {

As is this one.

>@@ -2758,17 +2714,18 @@ nsTableFrame::OrderRowGroups(FrameArray&
>+    nsTableRowGroupFrame* rowGroup =
>+      static_cast<nsTableRowGroupFrame*>(kidFrame);
>     if (NS_LIKELY(rowGroup)) {

And this one.

> nsTableFrame::GetTHead() const
>+      nsTableRowGroupFrame* rg = static_cast<nsTableRowGroupFrame*>(kidFrame);
>       if (rg) {

And this one.

> nsTableFrame::GetTFoot() const
>+      nsTableRowGroupFrame* rg = static_cast<nsTableRowGroupFrame*>(kidFrame);
>       if (rg) {

And this one.

> nsTableFrame::DumpRowGroup(nsIFrame* aKidFrame)
>+  if (!aKidFrame)
>+    return;
>+  
>+  if (aKidFrame) {

That makes no sense... ;)

Otherwise looks ok; can you find the various bugs we have on this scrollable rowgroup stuff and mark them as deps of this one?
Comment 40 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-14 23:18:42 PDT
Comment on attachment 386214 [details] [diff] [review]
patch to remove scrollable rowgroups

per comments
Comment 41 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-20 07:26:25 PDT
Note that I'm touching some of this code in bug 281387.  I'm pretty happy to have you land this first and merge on my end.
Comment 42 Bernd 2009-07-23 23:08:29 PDT
Created attachment 390420 [details] [diff] [review]
revised patch

there are two nontrivial changes: 
- I removed the code duplication of OrderedRowGroups. 
- I changed  ResetRowIndices to a Array-based skip mechanism where hits are removed to ensure that we do not search the same items again ( that should deal with the O(N*M)
Comment 43 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-27 11:20:24 PDT
Bernd, can I see a diff -w for that last patch?  It'd be a lot easier to review that way....
Comment 44 Bernd 2009-07-27 12:33:20 PDT
Created attachment 390868 [details] [diff] [review]
 hg wdiff -r qparent -r qtip
Comment 45 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-27 20:17:00 PDT
> that should deal with the O(N*M)

I don't see why...  If we're inserting near the end, we'll end up going through all of excludeRowGroups for every single rowgroup before the insert position, no?

If all we really need here is a quick test, wouldn't a hashset make more sense than an array?  An |nsTHashtable< nsPtrHashKey<nsIFrame> >| say?
Comment 46 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-07-27 20:29:40 PDT
Comment on attachment 390420 [details] [diff] [review]
revised patch

>+++ b/layout/tables/nsTableFrame.cpp

> void nsTableFrame::ResetRowIndices(nsIFrame* aFirstRowGroupFrame,
>+  RowGroupArray rowGroups;;

Nix the extra ';'

>+nsTableFrame::PushChildren(const RowGroupArray& aRowGroups,
>+  for (childX = aPushFrom; childX < aRowGroups.Length(); ++childX) {
>+      nsTableRowGroupFrame* rgFrame = aRowGroups[childX];
>+    // Don't push repeatable frames
>+    if (!rgFrame->IsRepeatable()) {

Weird indent on that first line in the loop.

With those nits and the O(N*M) thing fixed, r=bzbarsky
Comment 47 Bernd 2009-08-06 02:14:54 PDT
Created attachment 392897 [details] [diff] [review]
patch after framelist changes

Boris, could you please review the ResetRowIndices changes, that is the only significant change where I am uncertain if I did it right.
Comment 48 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-08-06 14:04:11 PDT
Comment on attachment 392897 [details] [diff] [review]
patch after framelist changes

The ResetRowIndices looks fine.

The whitespace nit from comment 46 is still not fixed.

Neither is the O(N*M) thing from comment 45.
Comment 49 Bernd 2009-10-04 11:33:38 PDT
Created attachment 404523 [details] [diff] [review]
hash table version
Comment 50 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-04 19:39:11 PDT
Comment on attachment 404523 [details] [diff] [review]
hash table version

No need for the RemoveEntry on excludeRowGroups: it won't make later hash lookups much faster, and it can make the whole procedure slower.

r=bzbarsky with that.
Comment 51 Bernd 2010-01-13 13:25:09 PST
Created attachment 421507 [details] [diff] [review]
patch updated to tip

not ready for checkin...

REFTEST TEST-UNEXPECTED-FAIL | file:///D:/moz_src/src/layout/reftests/table-back
ground/scrollable-rowgroup-separate-border.html | (!=)
Comment 53 Thomas 'PointedEars' Lahn 2011-02-11 12:10:10 PST
First of all, a rant: I must begin to question the sanity of this decision and the decision-makers.  The bug in Mac Firefox ought to be fixed, not a useful *and* *used* feature to be removed in all future Gecko-based browsers!  There was a time when Mozilla.org was to *set* and *push* standards, to bring innovation to the web.  Sorry to say that apparently that time has passed, starting with the ridiculous removal of MNG support for just a few KiB less a few years ago (no, it has not been forgotten). This is but another example of a step in the wrong direction.

Second, and much more important: If you really target (quasi-)standards (CSS 2.1 is not even a REC as of yet), you should know this:

If you remove this feature you are about to become non-compliant with CSS3 Box Model (WD) which says that the `overflow' property applies to "non-replaced block-level elements" whereas "A block-level box is a box that has a used value for ‘display’ of ‘block’, ‘list-item’, ‘table’, ‘table-*’ (i.e., all table boxes) or <template>." and "table-*" includes "table, inline-table, table-row-group, table-header-group, table-footer-group, table-row, table-column-group, table-column, table-cell, table-caption".  (It is not so hard to see that this is an erratum to CSS2.)  TBODY has display:table-row-group by default, and is therefore an element to which the `overflow' property applies.

http://www.w3.org/TR/css3-box/#overflow0
Comment 54 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-02-11 12:50:42 PST
The CSS3 Box text is very old (2007, if you note).  There have been updates to it since that just haven't been published yet.
Comment 55 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-02-11 12:52:30 PST
And to be clear, the mac issue here was not the main reason the for the removal.  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).
Comment 56 Thomas 'PointedEars' Lahn 2011-02-11 19:13:41 PST
(In reply to comment #54)
> The CSS3 Box text is very old (2007, if you note).

And?

> There have been updates to it since that just haven't been published yet.

Could you please back that up with a reference so that we can see how relevant those updates are to this problem?

(In reply to comment #55)
> And to be clear, the mac issue here was not the main reason the for the
> removal.  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).

I am not convinced.  This is a useful feature, and simply removing it is (yet another?) incentive to the editor to remove it from further draft versions as well because "nobody implements it".  Defining what is warranted or not is politics; deeming this feature not be warranted instead of fixing it ignores the people (like me) who have been using this feature on their web sites and suddenly find the behavior changed.  

One thing is for sure: If this feature is simply and silently going away, too, I will think twice before investing time into supporting an innovative Gecko feature again.
Comment 57 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-02-11 19:47:51 PST
> And?

And the point is that the text you're pointing to was considered a bug in the spec and that bug was fixed in the last 3 years.

> Could you please back that up with a reference

If I could, easily, I would.  You're welcome to search through the www-style archives like I'd have to, or you could wait for the next draft of CSS3 Box to be published....

> This is a useful feature,

Sure.  It was also fundamentally broken as implemented, and conflicted with what the CSS working group might want to do going forward.

> defining what is warranted or not is politics

No, in this case it was an engineering decision.  The support burden of this feature was too high compared to its benefits, especially when you put on the scales the fact that it didn't play nice with the standards process.

> ignores the people (like me) who have been using this feature on their web
> sites

And you had a fallback for cases when it wasn't supported, right?  Or did you just say "screw you" to other browsers?

> I will think twice before investing time into supporting an innovative Gecko
> feature again.

If the feature were implemented as "overflow: -moz-scrollable-table-body" there would have been no illusions about it being a "subject to change any time" spec extension, and people would have treated it as such, I hope.  But it wasn't, which was part of the problem with it.  I'd certainly hope that any future Gecko features will be better conceived and implemented than this one was.
Comment 58 lorenthal 2011-03-25 04:37:50 PDT
I agree completely with Thomas. I have a large web app deployed on more than 500 different servers. I've been convincing my users for years to use Firefox and abandon IE, for its better performance and all the advantadges it gives to us developers.

My app relies heavyly on this feature, with hundreds of tables (mostly dynamic) sparsed along the app. Obviously, as you said, we have a fallback action for other browsers, but it fails miserably with Firefox 4. On IE the fallback works, on Chrome or other webkit browsers the header is simply not fixed. But now Firefox 4 just display a portion of the table, without displaying any scrollbar. So for now, my app is completely unusable with Firefox 4.

As a took maker, you developers should take care of removing features that other developers could be using. Making all the changes needed in my app will take hours or days of manwork, deploying it to the hundreds of servers involved will take so much time that I don't want to think about it. 

You have deeply disappointed me (and, I'm sure, a lot of other developers who rely on this feature). You're doing the same that did Microsoft, and now everyone hates them. I'm sad to say that I will recommend my users to stay away from Firefox 4 and, perhaps, it will be easier for me to force them to use other browsers.

I'm a huge enthusiast of open-source, and I think that you're trying to do a good job, but you have to be much more carefull when you make changes that can break other people's apps. At last, if it wasn't for the huge community of developers who has been convincing users to change their browsers, Internet Explorer still would had a 99% market share.
Comment 59 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-03-25 10:23:54 PDT
It's odd that your fallback does different things in Chrome and Firefox.  Have you filed a bug on that?

> should take care of removing features that other developers could be using

We do.  The removal wasn't done lightly, I assure you.

> You're doing the same that did Microsoft,

What Microsoft did was to never remove features, precisely to avoid breaking people who were using its proprietary features and were therefore locked in.  They also couldn't add new features because adding them might break someone who depends on them not being present.  Not adding features and not fixing bugs are what everyone hates Microsoft for in the browser world.  And what you're saying is that we should do exactly the same thing as they were doing: keep our non-standard extensions in preference to implementing the standards.  Are you actually serious about that?
Comment 60 Les Barstow 2011-04-12 14:44:34 PDT
I've got to pile on a bit here.

I went looking to see if IE9 was going to support this very useful feature that implements a sadly missing and necessary feature of tables, and here I find the Firefox team, which led in supporting this feature - which worked great for my test cases anyway - is now dropping it because "the spec is 'old'", it's been "deprecated because no other browser supports it", and the published (if not finalized) spec may someday be replaced by an alternative.

The HTTP spec is how old?  It still works well, we aren't removing features that not all browsers support, and if there was a theoretical future change that might change things (e.g. SPDY) we're not ditching the current functionality just because it might eventually gain prominence.

If you're going to remove a feature, at least be kind enough to link over to these redesigns and the discussion where it appears they'll become more accepted than the current CSS3 spec which explicitly envisions this type of support.
Comment 61 Les Barstow 2011-04-12 14:46:54 PDT
PS - Don't know how to request that this issue be re-opened at least for a functional explanation of the resolution, but I'd like at least that for the developers who are understandably **** that it's gone.
Comment 62 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-04-12 15:27:45 PDT
Les, I think you misread something here.  The problem is not that the spec is "old" or that it might be replaced.  The spec says that this should not work, period.  The fact that we supported this was a violation of the spec.
Comment 63 Les Barstow 2011-04-12 15:35:11 PDT
I think I'm missing something here, yes.  The HTML 4 spec - a published, accepted spec last I checked - specifically notes of the THEAD, TBODY, and TFOOT elements that "This division enables user agents to support scrolling of table bodies independently of table head and foot."  And the obvious mechanism for this is via the overflow CSS - a mechanism apparently backed up by the (apparently aged but as yet unclarified) CSS3 spec itself.

That's what we all want here - a little snippet in the standard, proposed standard, or back-of-the-napkin multi-vendor agreement toward a standard that explains just what was so bad about this spec and why it should be different than it is.
Comment 64 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-04-12 15:51:16 PDT
CSS2.1, which is the only relevant spec on the CSS end here, explicitly says that the "overflow" property does not apply to table row groups.  

The "CSS3 Box Model" draft (not a spec!) which was referenced above says "overflow" applies to "block-level elements".  It also has a definition of "block-level elements" which has since been superseded by the CSS 2.1 definition (CSS 2.1 is newer than CSS 3 Box).  CSS 2.1 defines "block-level elements" as follows:

  The following values of the 'display' property make an element block-level:
  'block', 'list-item', and 'run-in' (part of the time; see run-in boxes), and
  'table'. 

CSS 3 Box will be updated with the same text once someone gets around to editing it again.  So in either case, "overflow" is not allowed to apply to row groups.
Comment 65 Ron MacCracken 2011-04-21 13:57:54 PDT
For what it's worth, I'd also like to ask that this "fix" be rethought. The application on which I work also made use of this very useful feature on our home-grown table widget that is used throughout our application, and now it is just gone! If there was another alternative, I could maybe understand removing the feature and recommending an alternative. However, the reason people are responding to this with such criticism in my opinion is that there is just no other good alternative. 

How would you recommend we support fixed headers in HTML, especially when table column widths are not fixed? This is typically done by having two tables (or even more divs) that communicate between each other to maintain their widths. We use the Dojo toolkit, and yes, we could start using their Grid widget that scrolls (we don't because we have a lot of infrastructure in place to prohibit that for now). However, do you see how much code and DOM structure is used behind the scenes to accomplish what it does? The same is true for other table widgets out there. Do you think that is really easier for widget makers to concoct all these elaborate schemes to try to make scrolling tables work, rather than for Mozilla to fix this issue once and make available for all?

We explicitly mentioned from the start that it was only supported on Firefox, so that users would not expect it to work elsewhere. The fallback position was that the feature was just not available and you get the full height of the table. I think this is perfectly fine for a browser application to do based on the different capabilities of each browser, especially when there is just no other alternative.

I'm even fine with it being a Mozilla extension. I don't really care. But either way we should have been left with some alternative.

It is just crazy to me that after 20 years of browsers, we are still waiting for things like scrolling tables, and even better, frozen columns. We have implemented our own version of it that works on all browsers (well, at least the ones we support: FF, IE, Chrome). It is not perfect, but works pretty well. But it sure would have been nice if the HTML table supported that too. If the HTML spec is to have a TABLE element, why not make it usable?

And I can't believe you would really need a spec to tell you how this should work. If you don't do it, other people will. The only problem is that they will all do it differently and you will have an infinitely harder problem to support 100 different variations than maybe 5 different browser vendors doing it.

Please reconsider!
Comment 66 j.j. 2011-04-21 15:48:03 PDT
(In reply to comment #65)
> And I can't believe you would really need a spec to tell you how this 
> should work. 
This was answered by yourself:
> The only problem is that they will all do it differently 
 
> Please reconsider!
Please read this bug completely (including it's dependencies). If you really feel strong about this issue, don't discuss it in this closed Bug. 
Your place is here:  http://lists.w3.org/Archives/Public/www-style/
Comment 67 Ron MacCracken 2011-04-21 18:06:56 PDT
Just a few more remarks and then I'll continue on the referenced site. Thanks for the reference.

(In reply to comment #66)
> (In reply to comment #65)
> > And I can't believe you would really need a spec to tell you how this 
> > should work.

After reading more on the subject referenced by the link you mentioned, I wish to take back the above remark. Reading the discussion did remind me that this is a complex issue and not easy to solve. That was not really my point though. My point was that there was a reasonable solution in place that worked for what I was doing (and apparently for others). So why not just keep that solution in place for people who wanted to continue using it, either as an extension or as it was?
 
> This was answered by yourself:
> > The only problem is that they will all do it differently 
> 

Not really. My point was that it is still better for the browser vendor to implement it once than for each developer to possibly come up with his/her own solution.

> > Please reconsider!
> Please read this bug completely (including it's dependencies). If you really
> feel strong about this issue, don't discuss it in this closed Bug. 
> Your place is here:  http://lists.w3.org/Archives/Public/www-style/
Comment 68 Alex Frase 2011-05-09 08:24:45 PDT
I think this is a case where the cure is worse than the disease.  How many people were really affected by this original bug?  How many users will be affected by the removal of this feature?  How many developers will be blamed for their applications "breaking" and have no good way to fix it?

The HTML and CSS specs are vague and can be interpreted in a number of ways; shouldn't the priority be on making the most things work for the most users?  Make it a proprietary -moz- attribute if you must, but simply removing a key feature and saying "tough, specs say no" is only going to alienate people.
Comment 69 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-09 08:27:41 PDT
> How many people were really affected by this original bug?

Every single Firefox user was affected by the security bugs in scrollable rowgroups.
Comment 70 George 2011-05-11 14:01:15 PDT
Have any decisions been made to reconsider this "fix".   Please reconsider so I can allow my users to migrate to 4.  Otherwise my users (hundreds) will remain where they are
Comment 71 Ryan Jones 2011-05-11 14:58:30 PDT
(In reply to comment #70)
> Have any decisions been made to reconsider this "fix".   Please reconsider
> so I can allow my users to migrate to 4.  Otherwise my users (hundreds) will
> remain where they are

Read comment 69. This isn't going to be undone as it caused numerous security bugs. Security is a higher priority than breaking things that shouldn't have used behaviour forbidden in the CSS anyway.
Comment 72 Les Barstow 2011-05-11 15:14:48 PDT
Wait a minute.  Now we've mutated from having a layout bug, to having a discussion about unpublished CSS revisions, to having serious security holes affecting every Firefox user.

I've just read through the entire comment thread and comment #69 is the first comment in this entire discussion that says anything about there being a security hole caused by this feature.

In fact, the most appropriate reason I can find for simply eliminating the scrolling row group feature is comment #37, wherein the developers have a gleeful IM discussion reveling in finding the CSS excuse to cut the feature.
Comment 73 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-11 18:07:52 PDT
Les, feel free to go read the history of the code in question, which just happens to be public; there have been numerous security issues in it.

Now can we please stop the whole "reintroduce this purposeful violation of the spec that doesn't work right anyway and is done in a braindead way that consumes maintenance resources out of proportion to its usefulness" discussion?  This feature is not coming back in its old form.  We'd like to figure out how to solve the actual problem at hand in a better way, but there has yet to be a sane suggestion for it.  Those who think they have them are free to mail www-style and suggest away.

As for people who force users to use an outdated (and soon to be insecure) browser, I'll refrain from speaking my mind for once.
Comment 74 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-11 18:09:41 PDT
And to be clear: there was not a _current_ security hole in the scrollable rowgroups code, to my knowledge (otherwise I wouldn't have brought up the topic).  But there were lots of past security holes, that took a significant amount of effort to deal with.  Effort that was not spent improving other areas of the browser, and hence pretty directly hurt all users of the browser.
Comment 75 Keith Spainhour 2011-07-26 06:30:15 PDT
I have raised Bug 674214 to get this functionality added back in.  Please go vote for it and add your comments.
Comment 76 Thomas 'PointedEars' Lahn 2011-08-20 13:54:29 PDT
The logic used for removing this feature is flawed.  Bug 674214 comment 6 shows why.  Please vote for that bug.  Thank you in advance.
Comment 77 Alan O. 2014-03-27 06:52:30 PDT
Official test document showing it is a feature in spec:

http://www.w3.org/WAI/UA/TS/html401/cp1001/1001-THEAD-TBODY-TFOOT-OVERFLOW.html

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