292756 - caption construction should use ConstructBlock

Status: RESOLVED WORKSFORME

(Core :: Layout: Tables, defect)

Description

[Bernd]

bz: in bug 291520
A better fix is to maybe use ConstructBlock here (note that I think passing in
the table creator as we do in ConstructTableCaptionFrame is wrong, so that's not
an obstacle to using ConstructBlock).  That would also have the quite salutary
(imo) effect of making overflow, columns, etc work on table captions.  It
_would_ mean that tables can't assume the thing on the caption list is an
nsTableCaptionFrame, though...  But I don't see any casts in the code to that
concrete class, so that may be ok.

I voted to avoid this for 1.8b2 and we went for a small crash fix.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So the problem is that code _does_ use the frame type to detect whether we
already have a caption (eg the code that prevents multiple captions).  So we
need to think about this a little bit.... how to do this right?

Comment 2

[Bernd]

One solution could be to avoid that frame creation suppression voodoo  at all by
adding every frame that wants to be a caption to the additional childlist, but
to render and consider for reflow only the first frame in that childist that has
display:table-caption.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Of course!  That should be easy enough to implement no?  We should do that, and
simplify all this code.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Note that when this is fixed we need to fix the code bug 307076 added accordingly.

Comment 5

[Bernd]

>A better fix is to maybe use ConstructBlock here (note that I think passing in
the table creator as we do in ConstructTableCaptionFrame is wrong

this has been fixed by David

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I believe we are constructing table caption via the same function that constructs block frame now. https://searchfox.org/mozilla-central/rev/f982032c7c7618c626165bb557968f478a1952dc/layout/base/nsCSSFrameConstructor.cpp#4276-4284,4296-4309