The default bug view has changed. See this FAQ.

Remove the ability for rowgroups to scroll (e.g. <tbody style="overflow:auto">)

RESOLVED FIXED

Status

()

Core
Layout: Tables
P3
normal
RESOLVED FIXED
17 years ago
3 years ago

People

(Reporter: Adam, Unassigned)

Tracking

({dev-doc-complete, testcase})

Trunk
dev-doc-complete, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [awd:tbl] [PATCH])

Attachments

(9 attachments, 2 obsolete attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Comment 1

17 years ago
Created attachment 5581 [details]
GIF of Table 5 footer problems
(Reporter)

Comment 2

17 years ago
Created attachment 5582 [details]
GIF of Table 5 BEFORE tscroll problem
(Reporter)

Comment 3

17 years ago
Created attachment 5583 [details]
GIF of Table 5 AFTER tscroll problems

Comment 4

17 years ago
The 1st problem is bug 26488.
Status: NEW → ASSIGNED
Target Milestone: M17
(Reporter)

Comment 5

17 years ago
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.
(Reporter)

Comment 6

17 years ago
Actually, I'm not seeing this anymore as of the 05.10.2000 build. Fixed?

Comment 7

17 years ago
Adding donttest keyword so this doesn't show up on the bugathon search page as
the URL is already a mozilla testcase.
Keywords: donttest

Comment 8

17 years ago
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. 
Target Milestone: M17 → Future
(Reporter)

Comment 9

17 years ago
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.
(Reporter)

Comment 10

17 years ago
Created attachment 15060 [details]
HTML file demonstrating problem
(Reporter)

Comment 11

17 years ago
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.
Summary: Problems w/ viewer demo #4 (simple tables) → Problem with layout -> <TFOOT> tag

Comment 12

17 years ago
Adam are you still seeing the problem? WFM Win98 CVS 2000-11-18.
OS: All
(Reporter)

Comment 13

17 years ago
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

17 years ago
Changing keyword from 'donttest' to 'testcase' as there is a testcase provided.
Keywords: donttest → testcase
nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 
Keywords: nsdogfood
Keywords: nsCatFood
Keywords: nsdogfood

Comment 16

16 years ago
QA contact update
QA Contact: chrisd → amar

Comment 17

16 years ago
Changing summary.
Summary: Problem with layout -> <TFOOT> tag → Problem with <TFOOT>, scrolling <TBODY>

Comment 18

16 years ago
Moving to m0.9.1
Target Milestone: Future → mozilla0.9.1

Comment 19

16 years ago
Created attachment 32972 [details] [diff] [review]
patch to allow space for vertical scroll bar

Comment 20

16 years ago
The only problem I'm seeing on Win2K is the clipped scrolling <tbody>, which the 
patch addresses.
Keywords: patch

Comment 21

16 years ago
Moving to m0.9.2
Target Milestone: mozilla0.9.1 → mozilla0.9.2

Comment 22

16 years ago
Moving to m0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Comment 23

16 years ago
*** Bug 74974 has been marked as a duplicate of this bug. ***

Comment 24

16 years ago
Missed 0.9.3.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Updated

16 years ago
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 25

16 years ago
reassigning to m0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Updated

16 years ago
Keywords: patch
Whiteboard: PATCH

Updated

16 years ago
Whiteboard: PATCH → [awd:tbl] [PATCH]

Comment 26

16 years ago
-->m098. The patch is a bit clugy and needs some re-thinking.
Target Milestone: mozilla0.9.6 → mozilla0.9.8

Comment 27

15 years ago
->m099
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Updated

15 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0.1

Updated

15 years ago
Target Milestone: mozilla1.0.1 → Future

Comment 28

15 years ago
*** Bug 101157 has been marked as a duplicate of this bug. ***

Comment 29

15 years ago
*** Bug 154301 has been marked as a duplicate of this bug. ***

Comment 30

14 years ago
mass reassign to default owner
Assignee: karnaze → table
Status: ASSIGNED → NEW
QA Contact: amar → madhur
Target Milestone: Future → ---

Updated

14 years ago
Target Milestone: --- → Future

Comment 31

14 years ago
*** Bug 205308 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Hardware: Macintosh → All
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.
Depends on: 173277
*** Bug 224078 has been marked as a duplicate of this bug. ***

Comment 34

13 years ago
*** Bug 187701 has been marked as a duplicate of this bug. ***

Comment 35

8 years ago
no other browser that I am aware scrolls the test case here, I think we should just remove the scrolling.

Comment 36

8 years ago
Created attachment 386214 [details] [diff] [review]
patch to remove scrollable rowgroups

Comment 37

8 years ago
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

8 years ago
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.
Attachment #386214 - Flags: superreview?(bzbarsky)
Attachment #386214 - Flags: review?(bzbarsky)
>+++ 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?
Attachment #386214 - Flags: superreview?(bzbarsky)
Attachment #386214 - Flags: superreview-
Attachment #386214 - Flags: review?(bzbarsky)
Attachment #386214 - Flags: review-
Comment on attachment 386214 [details] [diff] [review]
patch to remove scrollable rowgroups

per comments

Updated

8 years ago
Blocks: 135236

Updated

8 years ago
Blocks: 274473

Updated

8 years ago
Blocks: 308408
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.

Updated

8 years ago
Blocks: 317137

Updated

8 years ago
Blocks: 319167

Updated

8 years ago
Blocks: 387343

Updated

8 years ago
Blocks: 394518

Comment 42

8 years ago
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)
Attachment #386214 - Attachment is obsolete: true
Attachment #390420 - Flags: superreview?(bzbarsky)
Attachment #390420 - Flags: review?(bzbarsky)
Bernd, can I see a diff -w for that last patch?  It'd be a lot easier to review that way....

Comment 44

8 years ago
Created attachment 390868 [details] [diff] [review]
 hg wdiff -r qparent -r qtip
> 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 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
Attachment #390420 - Flags: superreview?(bzbarsky)
Attachment #390420 - Flags: review?(bzbarsky)
Attachment #390420 - Flags: review+

Comment 47

8 years ago
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.
Attachment #392897 - Flags: review?(bzbarsky)

Updated

8 years ago
Attachment #392897 - Attachment is patch: true
Attachment #392897 - Flags: review?(bzbarsky) → review-
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.
Assignee: layout.tables → nobody
QA Contact: madhur → layout.tables

Comment 49

8 years ago
Created attachment 404523 [details] [diff] [review]
hash table version
Attachment #392897 - Attachment is obsolete: true
Attachment #404523 - Flags: review?(bzbarsky)
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.
Attachment #404523 - Flags: review?(bzbarsky) → review+

Updated

7 years ago
Blocks: 532374
Blocks: 539342

Updated

7 years ago
Summary: Problem with <TFOOT>, scrolling <TBODY> → Remove the ability for rowgroups to scroll (e.g. <tbody style="overflow:auto">)
Target Milestone: Future → ---

Comment 51

7 years ago
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 52

7 years ago
http://hg.mozilla.org/mozilla-central/rev/e8350654c9bc
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Depends on: 558655

Updated

7 years ago
Keywords: dev-doc-complete
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
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.
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).
(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.
> 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

6 years ago
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.
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

6 years ago
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

6 years ago
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.
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

6 years ago
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.
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

6 years ago
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

6 years ago
(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

6 years ago
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

6 years ago
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.
> 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

6 years ago
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

6 years ago
(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

6 years ago
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.
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.
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

6 years ago
I have raised Bug 674214 to get this functionality added back in.  Please go vote for it and add your comments.
The logic used for removing this feature is flawed.  Bug 674214 comment 6 shows why.  Please vote for that bug.  Thank you in advance.

Updated

3 years ago
Flags: needinfo?

Comment 77

3 years ago
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
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.