Bug 286661 – All caption elements into a table are kept instead of just keeping the first one

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: Tables

[Karl Dubost]

See https://codepen.io/webcompat/pen/bNbzqJV

Safari and Chrome have the same issue.
Firefox shows only one caption. 


https://html.spec.whatwg.org/multipage/tables.html#the-caption-element

> As the first element child of a table element.


The table element is defined as:
https://html.spec.whatwg.org/multipage/tables.html#the-table-element

In this order: 
1. optionally a caption element, 
2. followed by zero or more colgroup elements, 
3. followed optionally by a thead element, 
4. followed by either zero or more tbody elements or one or more tr elements, 
5. followed optionally by a tfoot element, optionally intermixed with one or more script-supporting elements.








PS: Another thing to test separately 
> When a table element is the only content in a figure element other than the figcaption, the caption element should be omitted in favor of the figcaption.

[Karl Dubost]

Created attachment 474055 [details]
rendering in safari, firefox, chrome

Safari Technology Preview  212           20621.1.10.111.2
Firefox Nightly            136.0a1       13625.1.25
Google Chrome Canary       134.0.6985.0  6985.0

[Karl Dubost]

https://searchfox.org/wubkat/rev/47b66cb0dc7c2146fa7576555c720080708667f3/Source/WebCore/html/HTMLTableElement.cpp#169-176
```cpp
Ref<HTMLTableCaptionElement> HTMLTableElement::createCaption()
{
    if (auto existingCaption = caption())
        return existingCaption.releaseNonNull();
    auto caption = HTMLTableCaptionElement::create(captionTag, document());
    setCaption(caption.copyRef());
    return caption;
}
```


https://html.spec.whatwg.org/multipage/tables.html#dom-table-caption

==========
The caption IDL attribute must return, on getting, the **first caption element child of the table element**, if any, or null otherwise. 
On setting, the first caption element child of the table element, if any, must be removed, and the new value, if not null, must be inserted as the first node of the table element.

The createCaption() method must return 
  * the first caption element child of the table element, if any; 
  * otherwise a new caption element must be table-created, inserted as the first node of the table element, and then returned.

The deleteCaption() method must remove the first caption element child of the table element, if any.
==========


So the code needs to make sure that if the caption already exists it doesn't accumulate new elements.

[Karl Dubost]

This could be one of the reasons, WebKit fails
https://wpt.live/css/css-flexbox/alignment/flex-align-baseline-table-001.html

[Karl Dubost]

Or is it this part of the code, where it seems to take into account multiple captions.

https://searchfox.org/wubkat/rev/47b66cb0dc7c2146fa7576555c720080708667f3/Source/WebCore/rendering/RenderTable.cpp#409-420
```cpp
void RenderTable::layoutCaptions(BottomCaptionLayoutPhase bottomCaptionLayoutPhase)
{
    if (m_captions.isEmpty())
        return;
    // FIXME: Collapse caption margin.
    for (unsigned i = 0; i < m_captions.size(); ++i) {
        if ((bottomCaptionLayoutPhase == BottomCaptionLayoutPhase::Yes && m_captions[i]->style().captionSide() != CaptionSide::Bottom)
            || (bottomCaptionLayoutPhase == BottomCaptionLayoutPhase::No && m_captions[i]->style().captionSide() == CaptionSide::Bottom))
            continue;
        layoutCaption(*m_captions[i]);
    }
}
```

[Karl Dubost]

captions are defined as a vector. Not sure it should be. 

https://searchfox.org/wubkat/rev/47b66cb0dc7c2146fa7576555c720080708667f3/Source/WebCore/rendering/RenderTableCaption.cpp#40-44
```cpp
void RenderTableCaption::insertedIntoTree()
{
    RenderBlockFlow::insertedIntoTree();
    table()->addCaption(*this);
}
```

The code should just take the first one and ignore the other ones.


https://searchfox.org/wubkat/rev/47b66cb0dc7c2146fa7576555c720080708667f3/Source/WebCore/rendering/RenderTable.cpp#221-225
```cpp
void RenderTable::addCaption(RenderTableCaption& caption)
{
    ASSERT(m_captions.find(&caption) == notFound);
    m_captions.append(caption);
}
```


https://searchfox.org/wubkat/rev/47b66cb0dc7c2146fa7576555c720080708667f3/Source/WebCore/rendering/RenderTable.h#279
```cpp
mutable Vector<SingleThreadWeakPtr<RenderTableCaption>> m_captions;
``` 


Then there is `layoutCaptions()` with a S calling multiple time `layoutCaption()`

[Karl Dubost]

Created attachment 474079 [details]
testcase

[Karl Dubost]

I created https://github.com/whatwg/html/issues/10973

[karl]

Gecko has a bug. 
https://github.com/w3c/csswg-drafts/issues/11627
We can probably close here 

## CSS Tables Specification

In https://drafts.csswg.org/css-tables-3/#table-structure

> Optionally: one or more [table caption](https://drafts.csswg.org/css-tables-3/#table-caption).

Ok there can be more than one table caption. 

https://drafts.csswg.org/css-tables-3/#table-caption
> table-caption (equivalent to HTML: <caption>)
>    Specifies a caption for the table. Table captions are positioned between the table margins and its borders. 


https://drafts.csswg.org/css-tables-3/#caption-side-property seems to allude that Firefox has a bug related to multiple captions.
> Gecko has a bug when dealing with multiple captions. [!Testcase](http://codepen.io/FremyCompany/pen/WrJxwP)

And there is a WPT for it 
https://wpt.fyi/results/css/css-tables/caption-side-1.html?label=master&label=experimental&aligned&q=caption

[Karl Dubost]

It might also mean that Firefox should not pass 
https://wpt.live/css/css-flexbox/alignment/flex-align-baseline-table-001.html

[karl]

Ah maybe not because it's not checking the same thing.