commit 8b60f09bd9bd59d437f2acae123da8efaabe96c8
parent d4b20ea038f2f5f64e61859503e2e5ed9bca17b8
Author: Sajid Anwar <sajidanwar94@gmail.com>
Date: Tue, 6 Jan 2026 00:40:56 +0000
Bug 2008335 - Refactor HTML align attribute values and CSS property mappings. r=layout-reviewers,emilio
Another preliminary change to simplify the `vertical-align` shorthand conversion (D277725). It introduces a new enum called `HTMLAlignValue` designed to avoid the problem of having different CSS enum types in the attribute mappings for the `align` attribute.
In the process of implementing this, I found that the existing parsing and mapping of this attribute was convoluted and messy, with it not being immediately clear within a single element type how exactly the attribute value mapped to the CSS properties. (For example, the "div alignment" mapping was used for table elements, too, even though they had different parsing.) I added additional mapping methods so that each parser has a 1:1 correspondence with a mapper to avoid this confusion.
Try for this change: https://treeherder.mozilla.org/jobs?repo=try&revision=04da2213b1818b7b0285563f430871e48ad0d3da
Differential Revision: https://phabricator.services.mozilla.com/D277792
Diffstat:
7 files changed, 205 insertions(+), 125 deletions(-)
diff --git a/dom/html/HTMLTableCellElement.cpp b/dom/html/HTMLTableCellElement.cpp
@@ -178,8 +178,8 @@ void HTMLTableCellElement::MapAttributesIntoRule(
}
}
- nsGenericHTMLElement::MapDivAlignAttributeInto(aBuilder);
- nsGenericHTMLElement::MapVAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableCellHAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableVAlignAttributeInto(aBuilder);
nsGenericHTMLElement::MapBackgroundAttributesInto(aBuilder);
nsGenericHTMLElement::MapCommonAttributesInto(aBuilder);
}
diff --git a/dom/html/HTMLTableColElement.cpp b/dom/html/HTMLTableColElement.cpp
@@ -74,8 +74,8 @@ void HTMLTableColElement::MapAttributesIntoRule(
}
nsGenericHTMLElement::MapWidthAttributeInto(aBuilder);
- nsGenericHTMLElement::MapDivAlignAttributeInto(aBuilder);
- nsGenericHTMLElement::MapVAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableCellHAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableVAlignAttributeInto(aBuilder);
nsGenericHTMLElement::MapCommonAttributesInto(aBuilder);
}
diff --git a/dom/html/HTMLTableElement.cpp b/dom/html/HTMLTableElement.cpp
@@ -860,21 +860,6 @@ void HTMLTableElement::MapAttributesIntoRule(
aBuilder.SetPixelValue(eCSSProperty_border_spacing,
float(value->GetIntegerValue()));
}
- // align; Check for enumerated type (it may be another type if
- // illegal)
- value = aBuilder.GetAttr(nsGkAtoms::align);
- if (value && value->Type() == nsAttrValue::eEnum) {
- uint8_t enumValue = value->GetEnumValue();
-
- if (enumValue == uint8_t(StyleTextAlign::Center)) {
- aBuilder.SetAutoValueIfUnset(eCSSProperty_margin_left);
- aBuilder.SetAutoValueIfUnset(eCSSProperty_margin_right);
- } else if (enumValue == uint8_t(StyleTextAlign::Left)) {
- aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Left);
- } else if (enumValue == uint8_t(StyleTextAlign::Right)) {
- aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Right);
- }
- }
// bordercolor
value = aBuilder.GetAttr(nsGkAtoms::bordercolor);
@@ -905,6 +890,7 @@ void HTMLTableElement::MapAttributesIntoRule(
(float)borderThickness);
}
+ nsGenericHTMLElement::MapTableHAlignAttributeInto(aBuilder);
nsGenericHTMLElement::MapImageSizeAttributesInto(aBuilder);
nsGenericHTMLElement::MapBackgroundAttributesInto(aBuilder);
nsGenericHTMLElement::MapCommonAttributesInto(aBuilder);
diff --git a/dom/html/HTMLTableRowElement.cpp b/dom/html/HTMLTableRowElement.cpp
@@ -222,8 +222,8 @@ bool HTMLTableRowElement::ParseAttribute(int32_t aNamespaceID,
void HTMLTableRowElement::MapAttributesIntoRule(
MappedDeclarationsBuilder& aBuilder) {
nsGenericHTMLElement::MapHeightAttributeInto(aBuilder);
- nsGenericHTMLElement::MapDivAlignAttributeInto(aBuilder);
- nsGenericHTMLElement::MapVAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableCellHAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableVAlignAttributeInto(aBuilder);
nsGenericHTMLElement::MapBackgroundAttributesInto(aBuilder);
nsGenericHTMLElement::MapCommonAttributesInto(aBuilder);
}
diff --git a/dom/html/HTMLTableSectionElement.cpp b/dom/html/HTMLTableSectionElement.cpp
@@ -150,8 +150,8 @@ void HTMLTableSectionElement::MapAttributesIntoRule(
(float)value->GetIntegerValue());
}
}
- nsGenericHTMLElement::MapDivAlignAttributeInto(aBuilder);
- nsGenericHTMLElement::MapVAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableCellHAlignAttributeInto(aBuilder);
+ nsGenericHTMLElement::MapTableVAlignAttributeInto(aBuilder);
nsGenericHTMLElement::MapBackgroundAttributesInto(aBuilder);
nsGenericHTMLElement::MapCommonAttributesInto(aBuilder);
}
diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp
@@ -1103,13 +1103,33 @@ nsMapRuleToAttributesFunc nsGenericHTMLElement::GetAttributeMappingFunction()
return &MapCommonAttributesInto;
}
+// Represents a possible value for the "align" attribute. Different
+// elements may support different subsets of these values and map
+// them into different CSS properties.
+enum class HTMLAlignValue : uint8_t {
+ Left,
+ Right,
+ Top,
+ Middle,
+ Bottom,
+ Center,
+ Baseline,
+ TextTop,
+ AbsMiddle,
+ AbsCenter,
+ AbsBottom,
+ Justify,
+};
+
+// clang-format off
static constexpr nsAttrValue::EnumTableEntry kDivAlignTable[] = {
- {"left", StyleTextAlign::MozLeft},
- {"right", StyleTextAlign::MozRight},
- {"center", StyleTextAlign::MozCenter},
- {"middle", StyleTextAlign::MozCenter},
- {"justify", StyleTextAlign::Justify},
+ {"left", HTMLAlignValue::Left},
+ {"right", HTMLAlignValue::Right},
+ {"center", HTMLAlignValue::Center},
+ {"middle", HTMLAlignValue::Middle},
+ {"justify", HTMLAlignValue::Justify},
};
+// clang-format on
static constexpr nsAttrValue::EnumTableEntry kFrameborderTable[] = {
{"yes", FrameBorderProperty::Yes},
@@ -1137,62 +1157,31 @@ static constexpr nsAttrValue::EnumTableEntry kTableVAlignTable[] = {
};
static constexpr nsAttrValue::EnumTableEntry kAlignTable[] = {
- {"left", StyleTextAlign::Left},
- {"right", StyleTextAlign::Right},
-
- {"top", StyleVerticalAlignKeyword::Top},
- {"middle", StyleVerticalAlignKeyword::MozMiddleWithBaseline},
-
- // Intentionally not bottom.
- {"bottom", StyleVerticalAlignKeyword::Baseline},
-
- {"center", StyleVerticalAlignKeyword::MozMiddleWithBaseline},
- {"baseline", StyleVerticalAlignKeyword::Baseline},
-
- {"texttop", StyleVerticalAlignKeyword::TextTop},
- {"absmiddle", StyleVerticalAlignKeyword::Middle},
- {"abscenter", StyleVerticalAlignKeyword::Middle},
- {"absbottom", StyleVerticalAlignKeyword::Bottom},
+ {"left", HTMLAlignValue::Left},
+ {"right", HTMLAlignValue::Right},
+ {"top", HTMLAlignValue::Top},
+ {"middle", HTMLAlignValue::Middle},
+ {"bottom", HTMLAlignValue::Bottom},
+ {"center", HTMLAlignValue::Center},
+ {"baseline", HTMLAlignValue::Baseline},
+ {"texttop", HTMLAlignValue::TextTop},
+ {"absmiddle", HTMLAlignValue::AbsMiddle},
+ {"abscenter", HTMLAlignValue::AbsCenter},
+ {"absbottom", HTMLAlignValue::AbsBottom},
};
bool nsGenericHTMLElement::ParseAlignValue(const nsAString& aString,
nsAttrValue& aResult) {
- static_assert(uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::Top) &&
- uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::MozMiddleWithBaseline) &&
- uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::Baseline) &&
- uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::TextTop) &&
- uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::Middle) &&
- uint8_t(StyleTextAlign::Left) !=
- uint8_t(StyleVerticalAlignKeyword::Bottom));
-
- static_assert(uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::Top) &&
- uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::MozMiddleWithBaseline) &&
- uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::Baseline) &&
- uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::TextTop) &&
- uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::Middle) &&
- uint8_t(StyleTextAlign::Right) !=
- uint8_t(StyleVerticalAlignKeyword::Bottom));
-
return aResult.ParseEnumValue(aString, kAlignTable, false);
}
//----------------------------------------
static constexpr nsAttrValue::EnumTableEntry kTableHAlignTable[] = {
- {"left", StyleTextAlign::Left},
- {"right", StyleTextAlign::Right},
- {"center", StyleTextAlign::Center},
- {"justify", StyleTextAlign::Justify},
+ {"left", HTMLAlignValue::Left},
+ {"right", HTMLAlignValue::Right},
+ {"center", HTMLAlignValue::Center},
+ {"justify", HTMLAlignValue::Justify},
};
bool nsGenericHTMLElement::ParseTableHAlignValue(const nsAString& aString,
@@ -1204,12 +1193,12 @@ bool nsGenericHTMLElement::ParseTableHAlignValue(const nsAString& aString,
// This table is used for td, th, tr, col, thead, tbody and tfoot.
static constexpr nsAttrValue::EnumTableEntry kTableCellHAlignTable[] = {
- {"left", StyleTextAlign::MozLeft},
- {"right", StyleTextAlign::MozRight},
- {"center", StyleTextAlign::MozCenter},
- {"justify", StyleTextAlign::Justify},
- {"middle", StyleTextAlign::MozCenter},
- {"absmiddle", StyleTextAlign::Center},
+ {"left", HTMLAlignValue::Left},
+ {"right", HTMLAlignValue::Right},
+ {"center", HTMLAlignValue::Center},
+ {"justify", HTMLAlignValue::Justify},
+ {"middle", HTMLAlignValue::Middle},
+ {"absmiddle", HTMLAlignValue::AbsMiddle},
};
bool nsGenericHTMLElement::ParseTableCellHAlignValue(const nsAString& aString,
@@ -1400,61 +1389,153 @@ void nsGenericHTMLElement::MapImageAlignAttributeInto(
MappedDeclarationsBuilder& aBuilder) {
const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::align);
if (value && value->Type() == nsAttrValue::eEnum) {
- int32_t align = value->GetEnumValue();
- if (!aBuilder.PropertyIsSet(eCSSProperty_float)) {
- if (align == uint8_t(StyleTextAlign::Left)) {
+ switch (HTMLAlignValue(value->GetEnumValue())) {
+ case HTMLAlignValue::Left:
aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Left);
- } else if (align == uint8_t(StyleTextAlign::Right)) {
+ break;
+ case HTMLAlignValue::Right:
aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Right);
- }
- }
- if (!aBuilder.PropertyIsSet(eCSSProperty_vertical_align)) {
- switch (align) {
- case uint8_t(StyleTextAlign::Left):
- case uint8_t(StyleTextAlign::Right):
- break;
- default:
- aBuilder.SetKeywordValue(eCSSProperty_vertical_align, align);
- break;
- }
+ break;
+ case HTMLAlignValue::TextTop:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::TextTop);
+ break;
+ case HTMLAlignValue::Top:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Top);
+ break;
+ case HTMLAlignValue::Middle:
+ case HTMLAlignValue::Center:
+ aBuilder.SetKeywordValue(
+ eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::MozMiddleWithBaseline);
+ break;
+ case HTMLAlignValue::AbsMiddle:
+ case HTMLAlignValue::AbsCenter:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Middle);
+ break;
+ case HTMLAlignValue::AbsBottom:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Bottom);
+ break;
+ case HTMLAlignValue::Bottom: // Intentionally mapped to `baseline`
+ case HTMLAlignValue::Baseline:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Baseline);
+ break;
+ default:
+ MOZ_ASSERT_UNREACHABLE("Unexpected align value");
+ break;
}
}
}
void nsGenericHTMLElement::MapDivAlignAttributeInto(
MappedDeclarationsBuilder& aBuilder) {
- if (!aBuilder.PropertyIsSet(eCSSProperty_text_align)) {
- // align: enum
- const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::align);
- if (value && value->Type() == nsAttrValue::eEnum)
- aBuilder.SetKeywordValue(eCSSProperty_text_align, value->GetEnumValue());
+ const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::align);
+ if (value && value->Type() == nsAttrValue::eEnum) {
+ switch (HTMLAlignValue(value->GetEnumValue())) {
+ case HTMLAlignValue::Left:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozLeft);
+ break;
+ case HTMLAlignValue::Right:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozRight);
+ break;
+ case HTMLAlignValue::Center:
+ case HTMLAlignValue::Middle:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozCenter);
+ break;
+ case HTMLAlignValue::Justify:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::Justify);
+ break;
+ default:
+ MOZ_ASSERT_UNREACHABLE("Unexpected align value");
+ break;
+ }
}
}
-void nsGenericHTMLElement::MapVAlignAttributeInto(
+void nsGenericHTMLElement::MapTableVAlignAttributeInto(
MappedDeclarationsBuilder& aBuilder) {
- if (!aBuilder.PropertyIsSet(eCSSProperty_vertical_align)) {
- // align: enum
- const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::valign);
- if (value && value->Type() == nsAttrValue::eEnum) {
- switch (static_cast<TableCellAlignment>(value->GetEnumValue())) {
- case TableCellAlignment::Top:
- aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
- StyleVerticalAlignKeyword::Top);
- break;
- case TableCellAlignment::Middle:
- aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
- StyleVerticalAlignKeyword::Middle);
- break;
- case TableCellAlignment::Bottom:
- aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
- StyleVerticalAlignKeyword::Bottom);
- break;
- case TableCellAlignment::Baseline:
- aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
- StyleVerticalAlignKeyword::Baseline);
- break;
- }
+ const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::valign);
+ if (value && value->Type() == nsAttrValue::eEnum) {
+ switch (TableCellAlignment(value->GetEnumValue())) {
+ case TableCellAlignment::Top:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Top);
+ break;
+ case TableCellAlignment::Middle:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Middle);
+ break;
+ case TableCellAlignment::Bottom:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Bottom);
+ break;
+ case TableCellAlignment::Baseline:
+ aBuilder.SetKeywordValue(eCSSProperty_vertical_align,
+ StyleVerticalAlignKeyword::Baseline);
+ break;
+ }
+ }
+}
+
+void nsGenericHTMLElement::MapTableHAlignAttributeInto(
+ MappedDeclarationsBuilder& aBuilder) {
+ const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::align);
+ if (value && value->Type() == nsAttrValue::eEnum) {
+ switch (HTMLAlignValue(value->GetEnumValue())) {
+ case HTMLAlignValue::Center:
+ aBuilder.SetAutoValueIfUnset(eCSSProperty_margin_left);
+ aBuilder.SetAutoValueIfUnset(eCSSProperty_margin_right);
+ break;
+ case HTMLAlignValue::Left:
+ aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Left);
+ break;
+ case HTMLAlignValue::Right:
+ aBuilder.SetKeywordValue(eCSSProperty_float, StyleFloat::Right);
+ break;
+ default:
+ MOZ_ASSERT_UNREACHABLE("Unexpected align value");
+ break;
+ }
+ }
+}
+
+void nsGenericHTMLElement::MapTableCellHAlignAttributeInto(
+ MappedDeclarationsBuilder& aBuilder) {
+ const nsAttrValue* value = aBuilder.GetAttr(nsGkAtoms::align);
+ if (value && value->Type() == nsAttrValue::eEnum) {
+ switch (HTMLAlignValue(value->GetEnumValue())) {
+ case HTMLAlignValue::Left:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozLeft);
+ break;
+ case HTMLAlignValue::Right:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozRight);
+ break;
+ case HTMLAlignValue::Center:
+ case HTMLAlignValue::Middle:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::MozCenter);
+ break;
+ case HTMLAlignValue::AbsMiddle:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::Center);
+ break;
+ case HTMLAlignValue::Justify:
+ aBuilder.SetKeywordValue(eCSSProperty_text_align,
+ StyleTextAlign::Justify);
+ break;
+ default:
+ MOZ_ASSERT_UNREACHABLE("Unexpected align value");
+ break;
}
}
}
diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h
@@ -544,10 +544,23 @@ class nsGenericHTMLElement : public nsGenericHTMLElementBase {
static void MapDivAlignAttributeInto(mozilla::MappedDeclarationsBuilder&);
/**
- * Helper to map the valign attribute for things like <col>, <tr>, <section>.
+ * Helper to map the valign attribute for various table elements.
* @see GetAttributeMappingFunction
*/
- static void MapVAlignAttributeInto(mozilla::MappedDeclarationsBuilder&);
+ static void MapTableVAlignAttributeInto(mozilla::MappedDeclarationsBuilder&);
+
+ /**
+ * Helper to map the align attribute for <table>.
+ * @see GetAttributeMappingFunction
+ */
+ static void MapTableHAlignAttributeInto(mozilla::MappedDeclarationsBuilder&);
+
+ /**
+ * Helper to map the align attribute for various table elements.
+ * @see GetAttributeMappingFunction
+ */
+ static void MapTableCellHAlignAttributeInto(
+ mozilla::MappedDeclarationsBuilder&);
/**
* Helper to map the image border attribute.
