script: Clamp table spans according to the HTML specification (#36703)

Previously, spans were partially clamped during layout, but this means
that accessing and setting these properties via script wouldn't behave
according to the HTML specification. In addition, the value wasn't
floored in layout, so could lead to panics. This change improves
clamping and moves it to script.
    
Testing: This change includes a new WPT test.
Fixes #36699.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-05-01 15:25:34 +02:00 committed by GitHub
parent b10fc49e8a
commit 0d21992edd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 177 additions and 75 deletions

View file

@ -1019,15 +1019,16 @@ where
DisplayLayoutInternal::TableCell => { DisplayLayoutInternal::TableCell => {
// This value will already have filtered out rowspan=0 // This value will already have filtered out rowspan=0
// in quirks mode, so we don't have to worry about that. // in quirks mode, so we don't have to worry about that.
//
// The HTML specification limits the parsed value of `rowspan` to
// 65534 and `colspan` to 1000, so we also enforce the same limits
// when dealing with arbitrary DOM elements (perhaps created via
// script).
let (rowspan, colspan) = if info.pseudo_element_type.is_none() { let (rowspan, colspan) = if info.pseudo_element_type.is_none() {
let node = info.node.to_threadsafe(); let node = info.node.to_threadsafe();
let rowspan = node.get_rowspan().unwrap_or(1).min(65534) as usize; let rowspan = node.get_rowspan().unwrap_or(1) as usize;
let colspan = node.get_colspan().unwrap_or(1).min(1000) as usize; let colspan = node.get_colspan().unwrap_or(1) as usize;
// The HTML specification clamps value of `rowspan` to [0, 65534] and
// `colspan` to [1, 1000].
assert!((1..=1000).contains(&colspan));
assert!((0..=65534).contains(&rowspan));
(rowspan, colspan) (rowspan, colspan)
} else { } else {
(1, 1) (1, 1)
@ -1140,21 +1141,19 @@ fn add_column<'dom, Node: NodeExt<'dom>>(
is_anonymous: bool, is_anonymous: bool,
) -> ArcRefCell<TableTrack> { ) -> ArcRefCell<TableTrack> {
let span = if column_info.pseudo_element_type.is_none() { let span = if column_info.pseudo_element_type.is_none() {
column_info column_info.node.to_threadsafe().get_span().unwrap_or(1)
.node
.to_threadsafe()
.get_span()
.unwrap_or(1)
.min(1000) as usize
} else { } else {
1 1
}; };
// The HTML specification clamps value of `span` for `<col>` to [1, 1000].
assert!((1..=1000).contains(&span));
let column = ArcRefCell::new(TableTrack { let column = ArcRefCell::new(TableTrack {
base: LayoutBoxBase::new(column_info.into(), column_info.style.clone()), base: LayoutBoxBase::new(column_info.into(), column_info.style.clone()),
group_index, group_index,
is_anonymous, is_anonymous,
}); });
collection.extend(repeat(column.clone()).take(span)); collection.extend(repeat(column.clone()).take(span as usize));
column column
} }

View file

@ -70,13 +70,17 @@ impl HTMLTableCellElementMethods<crate::DomTypeHolder> for HTMLTableCellElement
make_uint_getter!(ColSpan, "colspan", DEFAULT_COLSPAN); make_uint_getter!(ColSpan, "colspan", DEFAULT_COLSPAN);
// https://html.spec.whatwg.org/multipage/#dom-tdth-colspan // https://html.spec.whatwg.org/multipage/#dom-tdth-colspan
make_uint_setter!(SetColSpan, "colspan", DEFAULT_COLSPAN); // > The colSpan IDL attribute must reflect the colspan content attribute. It is clamped to
// > the range [1, 1000], and its default value is 1.
make_clamped_uint_setter!(SetColSpan, "colspan", 1, 1000, 1);
// https://html.spec.whatwg.org/multipage/#dom-tdth-rowspan // https://html.spec.whatwg.org/multipage/#dom-tdth-rowspan
make_uint_getter!(RowSpan, "rowspan", DEFAULT_ROWSPAN); make_uint_getter!(RowSpan, "rowspan", DEFAULT_ROWSPAN);
// https://html.spec.whatwg.org/multipage/#dom-tdth-rowspan // https://html.spec.whatwg.org/multipage/#dom-tdth-rowspan
make_uint_setter!(SetRowSpan, "rowspan", DEFAULT_ROWSPAN); // > The rowSpan IDL attribute must reflect the rowspan content attribute. It is clamped to
// > the range [0, 65534], and its default value is 1.
make_clamped_uint_setter!(SetRowSpan, "rowspan", 0, 65534, 1);
// https://html.spec.whatwg.org/multipage/#dom-tdth-bgcolor // https://html.spec.whatwg.org/multipage/#dom-tdth-bgcolor
make_getter!(BgColor, "bgcolor"); make_getter!(BgColor, "bgcolor");
@ -174,23 +178,26 @@ impl VirtualMethods for HTMLTableCellElement {
match *local_name { match *local_name {
local_name!("colspan") => { local_name!("colspan") => {
let mut attr = AttrValue::from_u32(value.into(), DEFAULT_COLSPAN); let mut attr = AttrValue::from_u32(value.into(), DEFAULT_COLSPAN);
if let AttrValue::UInt(_, ref mut val) = attr { if let AttrValue::UInt(_, ref mut value) = attr {
if *val == 0 { // From <https://html.spec.whatwg.org/multipage/#dom-tdth-colspan>:
*val = 1; // > The colSpan IDL attribute must reflect the colspan content attribute. It is clamped to
} // > the range [1, 1000], and its default value is 1.
*value = (*value).clamp(1, 1000);
} }
attr attr
}, },
local_name!("rowspan") => { local_name!("rowspan") => {
let mut attr = AttrValue::from_u32(value.into(), DEFAULT_ROWSPAN); let mut attr = AttrValue::from_u32(value.into(), DEFAULT_ROWSPAN);
if let AttrValue::UInt(_, ref mut val) = attr { if let AttrValue::UInt(_, ref mut value) = attr {
if *val == 0 { // From <https://html.spec.whatwg.org/multipage/#dom-tdth-rowspan>:
let node = self.upcast::<Node>(); // > The rowSpan IDL attribute must reflect the rowspan content attribute. It is clamped to
let doc = node.owner_doc(); // > the range [0, 65534], and its default value is 1.
// rowspan = 0 is not supported in quirks mode // Note that rowspan = 0 is not supported in quirks mode.
if doc.quirks_mode() != QuirksMode::NoQuirks { let document = self.upcast::<Node>().owner_doc();
*val = 1; if document.quirks_mode() != QuirksMode::NoQuirks {
} *value = (*value).clamp(1, 65534);
} else {
*value = (*value).clamp(0, 65534);
} }
} }
attr attr

View file

@ -20,8 +20,6 @@ use crate::dom::node::Node;
use crate::dom::virtualmethods::VirtualMethods; use crate::dom::virtualmethods::VirtualMethods;
use crate::script_runtime::CanGc; use crate::script_runtime::CanGc;
const DEFAULT_SPAN: u32 = 1;
#[dom_struct] #[dom_struct]
pub(crate) struct HTMLTableColElement { pub(crate) struct HTMLTableColElement {
htmlelement: HTMLElement, htmlelement: HTMLElement,
@ -62,9 +60,11 @@ impl HTMLTableColElement {
impl HTMLTableColElementMethods<crate::DomTypeHolder> for HTMLTableColElement { impl HTMLTableColElementMethods<crate::DomTypeHolder> for HTMLTableColElement {
// <https://html.spec.whatwg.org/multipage/#attr-col-span> // <https://html.spec.whatwg.org/multipage/#attr-col-span>
make_uint_getter!(Span, "span", DEFAULT_SPAN); make_uint_getter!(Span, "span", 1);
// <https://html.spec.whatwg.org/multipage/#attr-col-span> // <https://html.spec.whatwg.org/multipage/#attr-col-span>
make_uint_setter!(SetSpan, "span", DEFAULT_SPAN); // > The span IDL attribute must reflect the content attribute of the same name. It is clamped
// > to the range [1, 1000], and its default value is 1.
make_clamped_uint_setter!(SetSpan, "span", 1, 1000, 1);
} }
pub(crate) trait HTMLTableColElementLayoutHelpers<'dom> { pub(crate) trait HTMLTableColElementLayoutHelpers<'dom> {
@ -96,11 +96,12 @@ impl VirtualMethods for HTMLTableColElement {
fn parse_plain_attribute(&self, local_name: &LocalName, value: DOMString) -> AttrValue { fn parse_plain_attribute(&self, local_name: &LocalName, value: DOMString) -> AttrValue {
match *local_name { match *local_name {
local_name!("span") => { local_name!("span") => {
let mut attr = AttrValue::from_u32(value.into(), DEFAULT_SPAN); let mut attr = AttrValue::from_u32(value.into(), 1);
if let AttrValue::UInt(_, ref mut val) = attr { if let AttrValue::UInt(_, ref mut val) = attr {
if *val == 0 { // From <https://html.spec.whatwg.org/multipage/#attr-col-span>:
*val = 1; // > The span IDL attribute must reflect the content attribute of the same name.
} // > It is clamped to the range [1, 1000], and its default value is 1.
*val = (*val).clamp(1, 1000);
} }
attr attr
}, },

View file

@ -317,6 +317,26 @@ macro_rules! make_uint_setter(
}; };
); );
#[macro_export]
macro_rules! make_clamped_uint_setter(
($attr:ident, $htmlname:tt, $min:expr, $max:expr, $default:expr) => (
fn $attr(&self, value: u32) {
use $crate::dom::bindings::inheritance::Castable;
use $crate::dom::element::Element;
use $crate::dom::values::UNSIGNED_LONG_MAX;
use $crate::script_runtime::CanGc;
let value = if value > UNSIGNED_LONG_MAX {
$default
} else {
value.clamp($min, $max)
};
let element = self.upcast::<Element>();
element.set_uint_attribute(&html5ever::local_name!($htmlname), value, CanGc::note())
}
);
);
#[macro_export] #[macro_export]
macro_rules! make_limited_uint_setter( macro_rules! make_limited_uint_setter(
($attr:ident, $htmlname:tt, $default:expr) => ( ($attr:ident, $htmlname:tt, $default:expr) => (

View file

@ -4916,6 +4916,13 @@
{} {}
] ]
], ],
"colspan-zero-crash.html": [
"a50586a5bfa7551724b838a0438339a34b4930fb",
[
null,
{}
]
],
"crashtests": { "crashtests": {
"caption-repaint-crash.html": [ "caption-repaint-crash.html": [
"6a024d0c1d7ef58da06a489d80d187bcb2a3e350", "6a024d0c1d7ef58da06a489d80d187bcb2a3e350",
@ -739709,14 +739716,14 @@
], ],
"processing-model-1": { "processing-model-1": {
"col-span-limits.html": [ "col-span-limits.html": [
"a4a425b9c1f70926c77ad3eb1b8a8a87a4655de9", "2a1ac80e65ad37a0e15ca383e67889b79f2308d0",
[ [
null, null,
{} {}
] ]
], ],
"span-limits.html": [ "span-limits.html": [
"cdfa61bbcdc06ea62b80d042440d55fb0c89a186", "798639b387562a54965d5283673a50773c2a3c49",
[ [
null, null,
{ {

View file

@ -1487,9 +1487,6 @@
[colgroup.tabIndex: IDL set to -2147483648] [colgroup.tabIndex: IDL set to -2147483648]
expected: FAIL expected: FAIL
[colgroup.span: setAttribute() to 2147483647]
expected: FAIL
[colgroup.span: setAttribute() to 2147483648] [colgroup.span: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -1499,9 +1496,6 @@
[colgroup.span: setAttribute() to 4294967296] [colgroup.span: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[colgroup.span: setAttribute() to 1001]
expected: FAIL
[colgroup.span: IDL set to 0] [colgroup.span: IDL set to 0]
expected: FAIL expected: FAIL
@ -2276,9 +2270,6 @@
[col.tabIndex: IDL set to -2147483648] [col.tabIndex: IDL set to -2147483648]
expected: FAIL expected: FAIL
[col.span: setAttribute() to 2147483647]
expected: FAIL
[col.span: setAttribute() to 2147483648] [col.span: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -2288,9 +2279,6 @@
[col.span: setAttribute() to 4294967296] [col.span: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[col.span: setAttribute() to 1001]
expected: FAIL
[col.span: IDL set to 0] [col.span: IDL set to 0]
expected: FAIL expected: FAIL
@ -5657,9 +5645,6 @@
[td.tabIndex: IDL set to -2147483648] [td.tabIndex: IDL set to -2147483648]
expected: FAIL expected: FAIL
[td.colSpan: setAttribute() to 2147483647]
expected: FAIL
[td.colSpan: setAttribute() to 2147483648] [td.colSpan: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -5669,9 +5654,6 @@
[td.colSpan: setAttribute() to 4294967296] [td.colSpan: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[td.colSpan: setAttribute() to 1001]
expected: FAIL
[td.colSpan: IDL set to 0] [td.colSpan: IDL set to 0]
expected: FAIL expected: FAIL
@ -5684,9 +5666,6 @@
[td.colSpan: IDL set to 1001] [td.colSpan: IDL set to 1001]
expected: FAIL expected: FAIL
[td.rowSpan: setAttribute() to 2147483647]
expected: FAIL
[td.rowSpan: setAttribute() to 2147483648] [td.rowSpan: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -5696,9 +5675,6 @@
[td.rowSpan: setAttribute() to 4294967296] [td.rowSpan: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[td.rowSpan: setAttribute() to 65535]
expected: FAIL
[td.rowSpan: IDL set to 2147483647] [td.rowSpan: IDL set to 2147483647]
expected: FAIL expected: FAIL
@ -7157,9 +7133,6 @@
[th.tabIndex: IDL set to -2147483648] [th.tabIndex: IDL set to -2147483648]
expected: FAIL expected: FAIL
[th.colSpan: setAttribute() to 2147483647]
expected: FAIL
[th.colSpan: setAttribute() to 2147483648] [th.colSpan: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -7169,9 +7142,6 @@
[th.colSpan: setAttribute() to 4294967296] [th.colSpan: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[th.colSpan: setAttribute() to 1001]
expected: FAIL
[th.colSpan: IDL set to 0] [th.colSpan: IDL set to 0]
expected: FAIL expected: FAIL
@ -7184,9 +7154,6 @@
[th.colSpan: IDL set to 1001] [th.colSpan: IDL set to 1001]
expected: FAIL expected: FAIL
[th.rowSpan: setAttribute() to 2147483647]
expected: FAIL
[th.rowSpan: setAttribute() to 2147483648] [th.rowSpan: setAttribute() to 2147483648]
expected: FAIL expected: FAIL
@ -7196,9 +7163,6 @@
[th.rowSpan: setAttribute() to 4294967296] [th.rowSpan: setAttribute() to 4294967296]
expected: FAIL expected: FAIL
[th.rowSpan: setAttribute() to 65535]
expected: FAIL
[th.rowSpan: IDL set to 2147483647] [th.rowSpan: IDL set to 2147483647]
expected: FAIL expected: FAIL

View file

@ -0,0 +1,10 @@
<!DOCTYPE html>
<link rel="help" href="https://github.com/servo/servo/issues/36699">
<link rel="author" href="mailto:fwang@igalia.com" title="Frédéric Wang">
<span id="span"></span>
<script>
let th = document.createElement("th");
span.replaceWith(th);
th.colSpan = 0;
</script>

View file

@ -39,12 +39,21 @@ These two must look the same, each having 2 cells in one row:
</table> </table>
<br> <br>
<table id=table3> <table id=table3>
<col span=1001> <col id="colspan-3" span=1001>
<tr> <tr>
<td colspan=1000><div class="square"></div></td> <td colspan=1000><div class="square"></div></td>
<td><div class="square"></div></td> <td><div class="square"></div></td>
</tr> </tr>
</table> </table>
<table>
<tr>
<td id="colspan-limit-test1" colspan=5></td>
<td id="colspan-limit-test2" colspan=0></td>
<td id="colspan-limit-test3" colspan=1000></td>
<td id="colspan-limit-test4" colspan=1001></td>
<td id="colspan-limit-test5" colspan=5555555></td>
</tr>
</table>
</main> </main>
<script> <script>
@ -56,4 +65,48 @@ test(() => {
assert_equals(table2.offsetWidth, 51, "table2 width"); assert_equals(table2.offsetWidth, 51, "table2 width");
assert_equals(table3.offsetWidth, 51, "table3 width"); assert_equals(table3.offsetWidth, 51, "table3 width");
}, "col span of 1001 must be treated as 1000"); }, "col span of 1001 must be treated as 1000");
test(() => {
let td = document.createElement("td");
td.colSpan = 5;
assert_equals(td.colSpan, 5);
td.colSpan = 0;
assert_equals(td.colSpan, 1);
td.colSpan = 1000;
assert_equals(td.colSpan, 1000);
td.colSpan = 1001;
assert_equals(td.colSpan, 1000);
td.colSpan = 555555;
assert_equals(td.colSpan, 1000);
}, "colspan must be clamped to [1, 1000] when set via script");
test(() => {
assert_equals(document.getElementById("colspan-limit-test1").colSpan, 5);
assert_equals(document.getElementById("colspan-limit-test2").colSpan, 1);
assert_equals(document.getElementById("colspan-limit-test3").colSpan, 1000);
assert_equals(document.getElementById("colspan-limit-test4").colSpan, 1000);
assert_equals(document.getElementById("colspan-limit-test5").colSpan, 1000);
}, "colspan must be clamped to [1, 1000] when parsing attributes");
test(() => {
let column = document.getElementById("colspan-3");
column.span = 5;
assert_equals(column.span, 5);
column.span = 0;
assert_equals(column.span, 1);
column.span = 1000;
assert_equals(column.span, 1000);
column.span = 1001;
assert_equals(column.span, 1000);
column.span = 555555;
assert_equals(column.span, 1000);
}, "column span must be clamped to [1, 1000] when set via script");
</script> </script>

View file

@ -29,6 +29,17 @@
<!-- We'll add another 65534 rows later --> <!-- We'll add another 65534 rows later -->
</table> </table>
<table>
<tr>
<td id="rowspan-limit-test1" rowspan=5></td>
<td id="rowspan-limit-test2" rowspan=0></td>
<td id="rowspan-limit-test3" rowspan=1000></td>
<td id="rowspan-limit-test4" rowspan=65534></td>
<td id="rowspan-limit-test5" rowspan=65535></td>
<td id="rowspan-limit-test6" rowspan=5555555></td>
</tr>
</table>
<script> <script>
var $ = document.querySelector.bind(document); var $ = document.querySelector.bind(document);
@ -63,4 +74,34 @@ test(() => {
assert_equals($("#d1").getBoundingClientRect().bottom, assert_equals($("#d1").getBoundingClientRect().bottom,
$("#d2").getBoundingClientRect().bottom); $("#d2").getBoundingClientRect().bottom);
}, "rowspan of 65535 must be treated as 65534"); }, "rowspan of 65535 must be treated as 65534");
test(() => {
let td = document.createElement("td");
td.rowSpan = 5;
assert_equals(td.rowSpan, 5);
td.rowSpan = 0;
assert_equals(td.rowSpan, 0);
td.rowSpan = 1000;
assert_equals(td.rowSpan, 1000);
td.rowSpan = 65534;
assert_equals(td.rowSpan, 65534);
td.rowSpan = 65535;
assert_equals(td.rowSpan, 65534);
td.rowSpan = 555555;
assert_equals(td.rowSpan, 65534);
}, "rowspan must be clamped to [0, 65534] when set via script");
test(() => {
assert_equals(document.getElementById("rowspan-limit-test1").rowSpan, 5);
assert_equals(document.getElementById("rowspan-limit-test2").rowSpan, 0);
assert_equals(document.getElementById("rowspan-limit-test3").rowSpan, 1000);
assert_equals(document.getElementById("rowspan-limit-test4").rowSpan, 65534);
assert_equals(document.getElementById("rowspan-limit-test5").rowSpan, 65534);
assert_equals(document.getElementById("rowspan-limit-test6").rowSpan, 65534);
}, "rowspan must be clamped to [0, 65534] when parsing attributes");
</script> </script>