Using 2/11 build on Win 95, Win NT, Win 98, Kinux, Mac8.5. Open URLs above. Expected results: (1) http://slip/projects/marvin/html/tables_caption_align_left.html: Caption should align to the left at the top of the table. (2) http://slip/projects/marvin/html/tables_caption_align_right.html: Caption should align to the right at the top of the table. Actual results: (1) and (2): Captions do not align to left or right.

We may not be supporting this for a while.

This has been cut as a 1.0 feature.

when was the cut decided and who decided it?

It was decided because (1) there is not time before 5/18 to complete it and (2) IE doesn't really support it yet either.

so does that mean it will go in at a later milestone? Or did Saito/Krock/Leger/Hofmann agree to the de-commitment?

ekrock, agreed? Feature coming out?

Yes, it's officially out for 1.0.

Verified LATER.

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

Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz} radars should now be considered deprecated in favour of keywords. I am *really* sorry about the spam...

Reopening and moving to Future...

The following URL is also visible outside netscape: http: //lxr.mozilla.org/seamonkey/source/layout/html/tests/table/marvin/tables_caption_align_left.html adding testcase keyword, as marvin cases seem to be valid tests that should not appear in the bugathon query.

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

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

Adding Keyword 4xp as IE5 handles both testcases correctly.

Removed 4xp keyword. 4xp means "Nav4 did this and we need to be compatible." Click the Keywords link to see the definitions.

from keywords.cgi: Product Parity bugs. A bug is a Product Parity bug if it occurs on Mozilla builds, but does not occur using the latest release of Netscape Communicator 4.x or *competitor products*. The bug does not occure at a competitor product. I hope they are still competitors???

I'm not sure when "competitor products" was added there or why. What I *think* they mean by competitor products in this context is situations like: suppose feature X worked in both Nav4 and IE4. Then we'd want it to work in Mozilla. But this doesn't mean that in the extremely rare cases that IE5.5 implements a standards feature that Mozilla currently doesn't (and such cases are outnumbered a thousand to one by cases where we support the standard and they don't--just think about DOM1/2, etc.) that such bugs should be marked 4xp--because Nav4x didn't actually support it. Hope that clarifies things. Best is to think of 4xp as a synonym for "compatibility with Nav4.x." If the keywords definition is causing confusion, it needs to be clarified.

ekrock: I disagree. I understood the 4xp keyword to mean exactly what the definition says -- we have a bug that a released browser does not. If this is not what was intended, then we have several hundred bugs to recheck. Don't forget Mozilla is almost a complete rewrite from the Nav 4.x series. From a mozilla.org point of view, Nav 4.x is just as relevant as WinIE, MacIE, Opera, Konqueror, et al. Why would mozilla.org worry about being better than the Nav 4 series in particular? IMHO 4xp should be reinstated on this bug.

Attaching a testcase (original ones have a broken link). Testing with Win 98, Nav and Netscape 6 do not render 'caption align="left"' or 'caption align="right"'. IE renders them differently that the spec indicates (they left or right align the caption at the top). See spec at: http://www.w3.org/TR/html4/struct/tables.html#h-11.2.2

Bug 63966 is a duplicate of this bug. I have made a clear and CSS/HTML 4.01 validated testcase. Also a screenshot from Opera 5.0 is provided. It really looks like you are waiting for february 16 2001 to come first!

as our goal is to create a browser which is as standard-compliant as possible, nominating for 0.9. doubt this would be a nsbeta1 candidate

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

CC:ing sorry for the spam

The dupe bug 10216 was dealing also with the caption-side property. Not implementing the caption-side property makes it impossible to create a table accordingly to CSS2 sect. 17.4.01.

QA contact update

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

removing myself from the cc list

This testcase is weird, cause if you remove the line changing the rules its stayed centered. Removing only the other line instead also makes the caption stay in the correct position.

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

I think this bug is caused by uncorrectly use parsent's nsHTMLReflowState. when nsTableOuterFrame reflow its children--nsTableCaptionFrame, it pass its nsHTMLReflowState to it, but nsTableCaptionFrame has its only style. So my patch is just calculate captionframes's align value, set captionReflowState's mStyleText according to it. I an new to reflow, any suggestion is welcome.

Comment on attachment 87332 [details] [diff] [review] patch 0.1 >+ if (alignValue.Equals(NS_LITERAL_STRING("left"))) styleText->mTextAlign = NS_STYLE_TEXT_ALIGN_LEFT; >+ if (alignValue.Equals(NS_LITERAL_STRING("right"))) styleText->mTextAlign = NS_STYLE_TEXT_ALIGN_RIGHT; >+ captionReflowState.mStyleText = styleText; Please change this to be: if (alignValue...) styleText->... else if (alignValue) styleText->...

Comment on attachment 87332 [details] [diff] [review] patch 0.1 This patch is completely incorrect, for the following reasons: 1. Caption alignment means that the caption should be on a different side of the table, not that its contents should be text-aligned. See http://www.w3.org/TR/html4/struct/tables.html#h-11.2.2 2. The attribute mapping of the alignment attributes is already done correctly, in MapAttributesIntoRule in nsTableCaptionElement.cpp, but they're mapped into the caption-side property (as they should be), not the text-align property. 3. The nsStyleText struct that you're trying to modify (and on which you're casting away const) is immutable -- it may be shared with other elements, and you don't want to modify the data for those elements.

I know my patch is not correct, so mark it as obsolete. but to dbaron's comment,I have some question about it. 1.so what do you think of this testcase should be? <TABLE border="1" width="200" ><CAPTION align="right">A test table </CAPTION><TR><TD>123</TABLE></HTML> Using IE, the text "A test table" will be placed on the top of table,right aligned. but with mozilla, the text "A test table" will be placed on the top center of table. 2.So how to understand the meaning of "left: The caption is at the left of the table." in html4 spec. before my understanding is that the text of caption will be left aligned.

No, it means that the entire caption will be to the left of the table. IE is probably incorrect. In other words, attachment 22369 [details] should look like: The table's Data Data caption. Data Data (and the 'caption-side: right' there is equivalent to the align="right" attribute on a caption element).

i got one idea about it. since mozilla, opera do not support caption-align, and IE implenments this in its own way. can mozilla actions just like IE in quirk mode?

We should consider quirks in quirks mode if the quirk is needed to fix the display of real pages on the web. (My typical definition of real pages is those where the bugs are reported by someone other than the author of the page.) Is this feature actually used on the web? I doubt it.

I go with dbaron's oppinion. We have the groundwork to get this working as specified in the spec already in nsTableOuterFrame.cpp. And once I have my buglist shrinked I will certainly go to implement this correctly. So please don't hork that with a quirk.

it looks to me like the w3c will drop caption-side:left and right from the css 2.1 spec.

... but only for lack of implementation. It doesn't make this bug any less valid.

I have a running prototype, taking the bug.

bernd, in your implementation proposal img, I know its (x,y, w), but don't know its height, maybe its h can be the same as Table body, but it will be ugly if it's a big table. maybe its h can be the first row height? I dont know.

The fun part is what to do for 'auto' widths on right and left captions. :-) The optimal solution, would, I think, be to use the minimum width that doesn't make the caption taller than the table, unless the caption wouldn't fit horizontally in such a width. That's quite hard, though, and I'm not sure what the implementable alternative should be.

David, I do my mozilla work only for fun (TM). The issue that I have, which worries me, is that, at http://www.people.fas.harvard.edu/~dbaron/css/test/sec1704 the last cases imply a different margin handling then attachment 98261 [details]. While I believe in my proposal, I can't see how in your margin handling one could center a table and keep the caption attached directly to the table. Second, wouldn't, if even if you can do that, the caption shift the center of gravity of the whole construct to the left or right. Just imagine a windmil testcase where the table is centered and via DOM the caption is circled around the table. In my proposal the table would keep its position horizontally and only move vertically depending whether the caption is top or not. Any insight from you is very wellcome, I would you anyway ask you for sr once the thing is working correctly.

This patch is a snapshot of my current tree, the following things need still to be done: DOM manipulations regression tests

The patch passed the regression tests. Its open for discussion even r/sr.

Bernd, you've done a great job with this. The patch I'm attaching corrects some cosmetics, reduces the amount of code in a few places, and add "XXX BERND" comments when I had a question or thought there might be a problem. I didn't run this patch through the regression tests, but I don't think the changes I've made are serious enough to cause things to break. After you attach a new patch, I would like to review it.

Comment on attachment 103466 [details] [diff] [review] patch revisited there is an error

I just ran through some tests with the patch, and I must say that overall I'm very impressed, Bernd. However - I think you should use the caption's top/bottom margins in the vertical position calculation. e.g. caption { caption-side: left; vertical-align: top; margin-top: 5px; } The top border edge of the caption should be 5px below the top of the table. If the caption has "vertical-align: center", I think it should be 2.5px off-center. (The margin would change its "center of gravity", as you say. If it had "margin: 5px", the caption would be once again exactly centered.) Of course, that's open to interpretation. I would like some future version of CSS to allow percentage values for vertical-align so that captions may be aligned vertically in a manner similar to background images. Any particular reason why centered captions longer than the table aren't centered? Percentage heights aren't recognized. Percentage widths give strange results. I'll attach a testcase.

I think percent width on a side-aligned caption should be relative to the table's margin (i.e. available space) rather than the containing block. You can get the latter effect by using auto width and a % margin on the table.

I left the vertical margins intentionaly out now. I see them conflicting with the vertical alignment and will implement that later. This patch does exactly what is written in the CSS2.0 spec for vertical alignment. If you look at your proposal it looks more like a proposal for w3c-style then something one should implement now. We should go for clarification once this patch is in and there is something to show to the w3c. >Any particular reason why centered captions longer than the table aren't >centered? could you provide a testcase, I don't see that. >Percentage heights aren't recognized. This is not the case, see the attached testcase >I think percent width on a side-aligned caption should be relative to the >table's margin (i.e. available space) rather than the containing block. As I explained on IRC I disagree the CSS2 specs states clearly: The percentage is calculated with respect to the width of the generated box's containing block. And we do so for the top and bottom already. I would not change that, because I think it is wrong.

the patch fixes attachment 104284 [details]

The caption disappears when width <= negative margin The disappearing text in the fifth test happens when width is given in pixels, but not when it's given in em.

I built with the latest patch. The centered caption testcase and the one I just attached both work fine. However, the "caption % width testcase" has problems with the right margin: It's still as wide as it was before the caption was added, so the canvas is too wide. >>Percentage heights aren't recognized. >This is not the case, see the attached testcase What is the containing block for a table caption that is neither above nor below the table? Is it the anonymous table+caption box, or that box's containing block? I was just reading through the CSS2 section on table captions, and it seems that putting the caption in the side margins doesn't quite match the wording. (See 2nd paragraph under 17.4 and the caption-in-margin example.) I really like what you've done here, and comment 50 has a very compelling argument for this model. Still, I thought I should bring this up...

Comment on attachment 104307 [details] [diff] [review] patch r=karnaze Bernd, I assume that you have a bunch of test cases to check in as regression tests. Here are a few additional comments. The indentation is 2px too much + switch (captionSide) { + case NS_SIDE_LEFT: Chage + if(mCaptionFrame) { to + if (mCaptionFrame) { Change * There are for possible scenarios to * There are four possible scenarios Change (2 places) default: // all others are treated as top for now to default: // top The following occurs in 2 places, can you make it a function. Also, could you check that there isn't other similar code sections that are duplicated. + if (NS_SIDE_LEFT == captionSide || NS_SIDE_RIGHT == captionSide) { + if (mCaptionFrame) { + nsMargin capMarginIgnore; + nsMargin capMarginNoAuto; + nsMargin captionPaddingIgnore; + GetMarginPadding(aPresContext, aOuterRS, mCaptionFrame, aOuterRS.availableWidth, capMarginIgnore, + capMarginNoAuto, captionPaddingIgnore); + PRBool isPctWidth; + IsAutoWidth( *mCaptionFrame,&isPctWidth); + if (isPctWidth && (eReflowReason_Initial != aOuterRS.reason)) { + nsRect priorCaptionRect; + mCaptionFrame->GetRect(priorCaptionRect); + capMin = priorCaptionRect.width; + } + capMin += capMarginNoAuto.left + capMarginNoAuto.right; + } + }

Comment on attachment 104633 [details] [diff] [review] patch with reviewers changes taking over the r=karnaze

Comment on attachment 104633 [details] [diff] [review] patch with reviewers changes sr=kin@netscape.com Just some minor formatting nits ... ==== Removing the following will leave 3 blank lines between the 2 left over functions, remove 2 of them to match the spacing between functions used in the file. -// tables change 0 width into auto, trees override this and do nothing -NS_IMETHODIMP -nsTableOuterFrame::AdjustZeroWidth() -{ - /* It does not appear that this function is needed any longer, since - the content node handles the deprecated width="0" attribute case. -- dwh - */ - return NS_OK; -} + ==== I see comments like this throughout the diff, where the comment is in the middle of the function arg list? Are these comments necessary since the code that sets the NoAuto vars looks self explanatory? + // aMargin, but with auto margins set to 0 + // aInnerMargin, but with auto margins set to 0 ==== Removing GetChildAvailWidth() leaves 2 blank lines above MoveFrameTo(), remove one of the lines. ==== Are we keeping this comment for a reason? Or can it be removed? + nsHTMLReflowMetrics captionMet((eReflowReason_StyleChange != reflowReason) ? nsnull : &maxElementSize); + //nsHTMLReflowMetrics captionMet(nsnull); // don't ask for MES, it hasn't changed

fix checked in, btek went from 1070 to 1069ms :-) Please open new bugs if you find bugs within this implementation.

great work Bernd !

Comment on attachment 105557 [details] new test case showing that layout stays correct of the caption-side is dynamically changed you don't need to attach a new file just to change the mimetype. next time click edit in the attachments area, uncheck [x] patch and enter a new mimetype!

Verified on trunk Build: 2002112108. All the testcases works fine for " caption align".