LibWeb: Support cellpadding=0 and invalidate table cells when it changes

We were incorrectly treating cellpadding=0 as if the attribute was
missing. This commit fixes it so it behaves as `padding: 0` on cells.

When adding a test, I discovered that we were not invalidating style for
cells when their containing table's cellpadding attribute changed.
So this commit fixes that as well.
This commit is contained in:
Andreas Kling 2025-02-21 00:43:26 +01:00 committed by Tim Ledbetter
parent fe2dff4944
commit 20c859519b
Notes: github-actions[bot] 2025-02-21 01:17:42 +00:00
6 changed files with 44 additions and 15 deletions

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2022, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2020-2025, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2024, Jamie Mansfield <jmansfield@cadixdev.org>
*
* SPDX-License-Identifier: BSD-2-Clause
@ -99,11 +99,11 @@ void HTMLTableCellElement::apply_presentational_hints(GC::Ref<CSS::CascadedPrope
if (!table_element)
return;
if (auto padding = table_element->padding()) {
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingTop, CSS::LengthStyleValue::create(CSS::Length::make_px(padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingBottom, CSS::LengthStyleValue::create(CSS::Length::make_px(padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingLeft, CSS::LengthStyleValue::create(CSS::Length::make_px(padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingRight, CSS::LengthStyleValue::create(CSS::Length::make_px(padding)));
if (auto padding = table_element->cellpadding(); padding.has_value()) {
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingTop, CSS::LengthStyleValue::create(CSS::Length::make_px(*padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingBottom, CSS::LengthStyleValue::create(CSS::Length::make_px(*padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingLeft, CSS::LengthStyleValue::create(CSS::Length::make_px(*padding)));
cascaded_properties->set_property_from_presentational_hint(CSS::PropertyID::PaddingRight, CSS::LengthStyleValue::create(CSS::Length::make_px(*padding)));
}
auto border = table_element->border();

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2020-2025, Andreas Kling <andreas@ladybird.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2020-2025, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2021, Adam Hodgen <ant1441@gmail.com>
*
* SPDX-License-Identifier: BSD-2-Clause
@ -16,6 +16,7 @@
#include <LibWeb/DOM/Document.h>
#include <LibWeb/DOM/ElementFactory.h>
#include <LibWeb/DOM/HTMLCollection.h>
#include <LibWeb/HTML/HTMLTableCellElement.h>
#include <LibWeb/HTML/HTMLTableColElement.h>
#include <LibWeb/HTML/HTMLTableElement.h>
#include <LibWeb/HTML/HTMLTableRowElement.h>
@ -145,11 +146,20 @@ void HTMLTableElement::attribute_changed(FlyString const& name, Optional<String>
Base::attribute_changed(name, old_value, value, namespace_);
if (name == HTML::AttributeNames::cellpadding) {
auto old_cellpadding = m_cellpadding;
if (value.has_value())
m_padding = max(0, parse_integer(value.value()).value_or(0));
m_cellpadding = max(0, parse_integer(value.value()).value_or(0));
else
m_padding = 1;
m_cellpadding = 1;
// NOTE: cellpadding is magical, it applies to the cells inside this table, not the table itself.
// When it changes, we need new style for the cells.
if (old_cellpadding != m_cellpadding) {
for_each_in_subtree_of_type<HTMLTableCellElement>([&](auto& cell) {
cell.set_needs_style_update(true);
return TraversalDecision::Continue;
});
}
return;
}
}
@ -478,9 +488,9 @@ unsigned int HTMLTableElement::border() const
return parse_border(get_attribute_value(HTML::AttributeNames::border));
}
unsigned int HTMLTableElement::padding() const
Optional<u32> HTMLTableElement::cellpadding() const
{
return m_padding;
return m_cellpadding;
}
}

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2020-2025, Andreas Kling <andreas@ladybird.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -48,7 +48,7 @@ public:
virtual Optional<ARIA::Role> default_role() const override { return ARIA::Role::table; }
unsigned border() const;
unsigned padding() const;
[[nodiscard]] Optional<u32> cellpadding() const;
private:
HTMLTableElement(DOM::Document&, DOM::QualifiedName);
@ -64,7 +64,7 @@ private:
GC::Ptr<DOM::HTMLCollection> mutable m_rows;
GC::Ptr<DOM::HTMLCollection> mutable m_t_bodies;
unsigned m_padding { 1 };
Optional<u32> m_cellpadding;
};
}

View file

@ -0,0 +1,4 @@
1px
5px
0px
0px

View file

@ -0,0 +1,15 @@
<script src="../include.js"></script>
<table id="table"><td id="td"></td></table>
<script>
function testCellpadding(value) {
table.setAttribute("cellpadding", value);
println(getComputedStyle(td).padding);
}
test(() => {
testCellpadding("1");
testCellpadding("5");
testCellpadding("0");
testCellpadding("-1");
});
</script>