Table cellpadding attribute #12609

adamncasey · 2016-07-26T23:40:13Z

Although fairly legacy, it probably makes sense for servo to support the cellpadding attribute of <table>.

<html>
<head>
    <title>Table cellpadding attribute + padding CSS</title>
    <style type="text/css">
        table { background-color: red; }
        div { width:10px;height:10px; }
    </style>
</head>
<body>
    <table>
        <tr>
            <td style="padding:5px"><div></div></td>
        </tr>
    </table>
    <table cellpadding="5">
        <tr>
            <td><div></div></td>
        </tr>
    </table>
</body>
</html>

The above example renders as a single column of red in Firefox, Chrome & Edge.

https://html.spec.whatwg.org/multipage/rendering.html#tables-2 describes how cellpadding should be interpreted.

Related to #11821 in the sense that this prevents HN's top bar from being rendered correctly.

I'm hoping to give this a go by following how cellspacing was implemented in #4417 . Some of that has been moved around quite a bit since though.

adamncasey · 2017-08-15T03:13:26Z

Picked this up again recently. Current WIP handles the example above correctly. https://github.com/adamncasey/servo/tree/feature/cellpadding

It turned out that cellspacing is somewhat different to cellpadding since cellpadding is actually applied to the cells in the table, not the table itself.

The current implementation looks up the table's cellspacing value when synthesize_presentational_hints_for_legacy_attributes is called on the cell elements. It looks it up by asking for the parent and following chain: Cell -> Row -> Section -> Table. There's no recursion or iteration here, but it's quite verbose code.

Another possible implementation is to pass some kind of reference down this chain as each object is created. The reference needs to be mutable by the table, as this property can be updated. This would probably increase memory by a few bytes per table section/row/cell with the benefit that each time this synthesize_presentational_hints_for_legacy_attributes call is made we could skip two hops. I'm not sure when/how often this function is called yet.

HN Top bar after/before patch:

Aiming to put in PR in the next few days

nox · 2017-10-04T11:18:00Z

#18161 is a PR for that.

jdm added A-content/dom A-layout/table labels Jul 27, 2016

This was referenced Aug 15, 2017

Top bar on Hacker News too large #17513

Closed

Add support for legacy table attribute cellpadding #18161

Open

servo / servo

Table cellpadding attribute #12609

Table cellpadding attribute #12609

adamncasey commented Jul 26, 2016 •

edited

adamncasey commented Aug 15, 2017

nox commented Oct 4, 2017

servo / servo

Join GitHub today

Table cellpadding attribute #12609

Table cellpadding attribute #12609

Comments

adamncasey commented Jul 26, 2016 • edited

adamncasey commented Aug 15, 2017

nox commented Oct 4, 2017

adamncasey commented Jul 26, 2016 •

edited