Skip to content

Commit ea47ec0

Browse files
OnionsYuOnionsYu
committed
Keep the topmost CSS background image with its fitting properties unescape quoted url paths and warn on conflicting data attributes.
1 parent bc045c7 commit ea47ec0

12 files changed

Lines changed: 172 additions & 46 deletions

‎src/pagx/PAGXOptimizer.cpp‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,15 +1255,27 @@ bool MergeAdjacentGroupsInLayer(Layer* layer) {
12551255
// proof of equality: parent and child may measure from different contents, and percentage-sized
12561256
// descendants would then resolve against a different box after reparenting. Require each axis to
12571257
// be either an exact concrete match or an explicit 100% fill.
1258+
bool IsZeroOrUnset(float value) {
1259+
return std::isnan(value) || value == 0.0f;
1260+
}
1261+
1262+
bool AxisFills(float childSize, float parentSize, float childPercent) {
1263+
// Percentage sizing takes precedence over the absolute field in constraint layout, so a
1264+
// programmatically-built node carrying both must be judged by its percentage.
1265+
if (!std::isnan(childPercent)) {
1266+
return childPercent == 100.0f;
1267+
}
1268+
return !std::isnan(childSize) && !std::isnan(parentSize) && childSize == parentSize;
1269+
}
1270+
12581271
bool ChildFillsParent(const Layer* parent, const Layer* child) {
12591272
if (child->x != 0.0f || child->y != 0.0f) {
12601273
return false;
12611274
}
12621275
if (!child->matrix.isIdentity() || !child->matrix3D.isIdentity()) {
12631276
return false;
12641277
}
1265-
auto zeroOrUnset = [](float v) { return std::isnan(v) || v == 0.0f; };
1266-
if (!zeroOrUnset(child->left) || !zeroOrUnset(child->top)) {
1278+
if (!IsZeroOrUnset(child->left) || !IsZeroOrUnset(child->top)) {
12671279
return false;
12681280
}
12691281
if (!std::isnan(child->right) || !std::isnan(child->bottom) || !std::isnan(child->centerX) ||
@@ -1274,18 +1286,8 @@ bool ChildFillsParent(const Layer* parent, const Layer* child) {
12741286
if (!parent->padding.isZero()) {
12751287
return false;
12761288
}
1277-
auto axisFills = [](float childSize, float parentSize, float childPercent) {
1278-
// Percentage sizing takes precedence over the absolute field in constraint layout, so a
1279-
// programmatically-built node carrying both must be judged by its percentage.
1280-
if (!std::isnan(childPercent)) {
1281-
return childPercent == 100.0f;
1282-
}
1283-
bool concreteMatch =
1284-
!std::isnan(childSize) && !std::isnan(parentSize) && childSize == parentSize;
1285-
return concreteMatch;
1286-
};
1287-
return axisFills(child->width, parent->width, child->percentWidth) &&
1288-
axisFills(child->height, parent->height, child->percentHeight);
1289+
return AxisFills(child->width, parent->width, child->percentWidth) &&
1290+
AxisFills(child->height, parent->height, child->percentHeight);
12891291
}
12901292

12911293
// The child applies nothing that would change how its payload is painted once it is reparented into

‎src/pagx/html/importer/HTMLBoxAttributes.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ static constexpr const char* HTML_DEFAULT_FONT_FAMILY = "Arial";
4646
*/
4747
static constexpr const char* HTML_DEFAULT_FONT_STYLE = "Regular";
4848

49+
/** Returns a concrete PAGX font style name for a resolved HTML face. */
50+
inline std::string ResolveHTMLFontStyleName(const std::string& fontStyleName) {
51+
return fontStyleName.empty() ? HTML_DEFAULT_FONT_STYLE : fontStyleName;
52+
}
53+
4954
/**
5055
* Default text colour used when no `color` is inherited; matches the CSS default for
5156
* <body> in `ElementDefaults()`.

‎src/pagx/html/importer/HTMLDetail.cpp‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ void ApplyBackgroundShorthand(const std::string& value,
205205

206206
auto layers = SplitTopLevelCommas(Trim(value));
207207
std::vector<std::string> imageLayers;
208+
bool hasURLImage = false;
208209
std::string colorToken;
209210
std::string positionValue;
210211
std::string sizeValue;
@@ -240,21 +241,32 @@ void ApplyBackgroundShorthand(const std::string& value,
240241
}
241242
}
242243
if (!imageToken.empty()) {
244+
bool isURLImage = ToLower(imageToken).rfind("url(", 0) == 0;
243245
imageLayers.push_back(std::move(imageToken));
244-
// PAGX currently consumes fitting properties only for a single url() background. Retaining
245-
// the last painted image layer matches the previous supported subset while preserving both
246-
// repeat axes when the CSS uses the two-keyword form.
247-
if (!positionTokens.empty()) positionValue = JoinStyleTokens(positionTokens);
248-
if (!sizeTokens.empty()) sizeValue = JoinStyleTokens(sizeTokens);
249-
if (!repeatTokens.empty()) repeatValue = JoinStyleTokens(repeatTokens);
246+
hasURLImage |= isURLImage;
247+
// PAGX can stack gradients, but its ImagePattern path consumes only one image layer. Keep
248+
// the fitting values from the CSS-topmost image so they stay paired with the same layer if
249+
// the shorthand has to be reduced to one image below.
250+
if (imageLayers.size() == 1) {
251+
positionValue = JoinStyleTokens(positionTokens);
252+
sizeValue = JoinStyleTokens(sizeTokens);
253+
repeatValue = JoinStyleTokens(repeatTokens);
254+
}
250255
}
251256
}
252257

253258
if (!imageLayers.empty()) {
254259
std::string joinedImages;
255-
for (const auto& image : imageLayers) {
256-
if (!joinedImages.empty()) joinedImages += ", ";
257-
joinedImages += image;
260+
if (hasURLImage) {
261+
// Mixed/url stacks cannot be represented by one PAGX ImagePattern. CSS paints the first
262+
// listed image on top, so retain that layer and its position/size/repeat values rather than
263+
// pairing a multi-layer image list with unrelated fitting data.
264+
joinedImages = imageLayers.front();
265+
} else {
266+
for (const auto& image : imageLayers) {
267+
if (!joinedImages.empty()) joinedImages += ", ";
268+
joinedImages += image;
269+
}
258270
}
259271
out["background-image"] = std::move(joinedImages);
260272
out["background-position"] = std::move(positionValue);

‎src/pagx/html/importer/HTMLElementEmitter.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,17 @@ std::string ExtractCssUrl(const std::string& value) {
165165
inner.back() == inner.front()) {
166166
inner = inner.substr(1, inner.size() - 2);
167167
}
168-
return Trim(inner);
168+
std::string unescaped;
169+
unescaped.reserve(inner.size());
170+
for (size_t i = 0; i < inner.size(); ++i) {
171+
if (inner[i] == '\\' && i + 1 < inner.size() &&
172+
(inner[i + 1] == '\\' || inner[i + 1] == '\'' || inner[i + 1] == '"')) {
173+
unescaped.push_back(inner[++i]);
174+
} else {
175+
unescaped.push_back(inner[i]);
176+
}
177+
}
178+
return Trim(unescaped);
169179
}
170180

171181
// Decodes a single hex digit to its 0-15 value, or -1 when not a hex digit.

‎src/pagx/html/importer/HTMLLayerBuilder.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,11 @@ void HTMLLayerBuilder::forwardDataAttributes(Layer* layer,
553553
if (attr.name.compare(0, 5, "data-") != 0) continue;
554554
std::string key = attr.name.substr(5);
555555
if (IsValidCustomDataKey(key)) {
556+
auto existing = layer->customData.find(key);
557+
if (existing != layer->customData.end() && existing->second != attr.value) {
558+
_diagnostics.warn("html: data-* attribute '" + attr.name +
559+
"' overrides an existing custom data value");
560+
}
556561
layer->customData[key] = attr.value;
557562
} else {
558563
_diagnostics.warn("html: invalid data-* attribute name '" + attr.name + "'");

‎src/pagx/html/importer/HTMLLayerBuilder.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ class HTMLLayerBuilder {
8080
void applyLayerAttributes(Layer* layer, const std::shared_ptr<DOMNode>& element,
8181
const HTMLBoxAttributes& box);
8282

83-
/** Copies the element's `data-*` attributes into `layer->customData` (invalid keys are warned
84-
* and dropped). Split out from `applyLayerAttributes` so the folded-image path — which reuses a
85-
* parent layer and skips the rest of the layer-attribute chain — can still forward the inner
86-
* `<img>`'s custom data. */
83+
/** Copies the element's `data-*` attributes into `layer->customData` (invalid keys and
84+
* conflicting overwrites are warned). Split out from `applyLayerAttributes` so the
85+
* folded-image path — which reuses a parent layer and skips the rest of the layer-attribute
86+
* chain — can still forward the inner `<img>`'s custom data. */
8787
void forwardDataAttributes(Layer* layer, const std::shared_ptr<DOMNode>& element);
8888

8989
/** Builds a `Fill` chain whose colour is a single solid `color`. */

‎src/pagx/html/importer/HTMLParserContext.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,7 @@ Layer* HTMLParserContext::convertElement(const std::shared_ptr<DOMNode>& element
355355
text->text = "\n";
356356
text->fontFamily = inherited.primaryFontFamily.empty() ? HTML_DEFAULT_FONT_FAMILY
357357
: inherited.primaryFontFamily;
358-
text->fontStyle =
359-
inherited.fontStyleName.empty() ? HTML_DEFAULT_FONT_STYLE : inherited.fontStyleName;
358+
text->fontStyle = ResolveHTMLFontStyleName(inherited.fontStyleName);
360359
text->fauxBold = inherited.fauxBold;
361360
text->fauxItalic = inherited.fauxItalic;
362361
text->fontSize = HTML_DEFAULT_FONT_SIZE;

‎src/pagx/html/importer/HTMLTextFragmentBuilder.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,7 @@ Text* HTMLTextFragmentBuilder::buildTextElement(const TextFragment& fragment) {
7272
auto t = _document->makeNode<Text>();
7373
t->text = fragment.text;
7474
t->fontFamily = fragment.fontFamily;
75-
// Every imported Text node must carry a concrete fontStyle. The synthesis leaves the label empty
76-
// for the plain base face (bold / italic ride on the faux flags), so substitute the canonical
77-
// "Regular" name here.
78-
t->fontStyle = fragment.fontStyleName.empty() ? HTML_DEFAULT_FONT_STYLE : fragment.fontStyleName;
75+
t->fontStyle = ResolveHTMLFontStyleName(fragment.fontStyleName);
7976
t->fauxBold = fragment.fauxBold;
8077
t->fauxItalic = fragment.fauxItalic;
8178
t->fontSize = fragment.fontSize;

‎test/src/PAGXHTMLImporterTest.cpp‎

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,28 @@ PAG_TEST(PAGXHTMLImporterTest, BackgroundShorthandColorPlusGradient) {
362362
EXPECT_TRUE(ColorNear(lg->colorStops.front()->color, HexColor(0xFF0000)));
363363
}
364364

365+
// PAGX represents a URL background with one ImagePattern. When a shorthand contains multiple
366+
// image layers, retain the CSS-topmost layer and the fitting properties declared on that same
367+
// layer instead of pairing the image with a lower layer's size/repeat/position.
368+
PAG_TEST(PAGXHTMLImporterTest, BackgroundShorthandKeepsTopImageFittingTogether) {
369+
auto doc = ParseFromString(R"HTML(
370+
<html><body style="width:160px;height:120px">
371+
<div style="width:160px;height:120px;
372+
background:url(top.png) center / contain no-repeat,
373+
url(bottom.png) left top / cover repeat"></div>
374+
</body></html>
375+
)HTML");
376+
ASSERT_NE(doc, nullptr);
377+
auto* layer = doc->layers.front()->children.front();
378+
auto* fill = FindElementOfType<pagx::Fill>(layer);
379+
ASSERT_NE(fill, nullptr);
380+
auto* pattern = As<pagx::ImagePattern>(fill->color);
381+
ASSERT_NE(pattern, nullptr);
382+
ASSERT_NE(pattern->image, nullptr);
383+
EXPECT_EQ(pattern->image->filePath, "top.png");
384+
EXPECT_EQ(pattern->scaleMode, pagx::ScaleMode::LetterBox);
385+
}
386+
365387
// A higher-priority shorthand resets every background longhand supplied by a lower-priority rule.
366388
// In particular, an inline colour-only background must clear a class-provided image.
367389
PAG_TEST(PAGXHTMLImporterTest, BackgroundShorthandResetsLowerPriorityLonghands) {
@@ -1277,14 +1299,10 @@ PAG_TEST(PAGXHTMLImporterTest, WhiteSpacePreKeepsSpacesVerbatim) {
12771299
"</body></html>");
12781300
ASSERT_NE(doc, nullptr);
12791301
auto* leaf = doc->layers.front()->children.front();
1302+
EXPECT_EQ(FindElementOfType<pagx::TextBox>(leaf), nullptr);
12801303
std::vector<pagx::Text*> texts;
12811304
std::vector<pagx::Fill*> fills;
1282-
if (auto* tb = FindElementOfType<pagx::TextBox>(leaf)) {
1283-
EXPECT_FALSE(tb->wordWrap);
1284-
GatherTextRuns(tb->elements, &texts, &fills);
1285-
} else {
1286-
GatherTextRuns(leaf->contents, &texts, &fills);
1287-
}
1305+
GatherTextRuns(leaf->contents, &texts, &fills);
12881306
ASSERT_FALSE(texts.empty());
12891307
const std::string& t = texts.front()->text;
12901308
EXPECT_NE(t.find("A B"), std::string::npos);
@@ -3146,6 +3164,30 @@ PAG_TEST(PAGXHTMLImporterTest, DataStarAttributesPropagateOnFoldedRoundedImage)
31463164
EXPECT_EQ(wrapper->customData["id"], "avatar");
31473165
}
31483166

3167+
// The folded <img> is the more specific payload node, so its data value wins over the structural
3168+
// wrapper's value. The lossy collision is surfaced as a diagnostic rather than silently ignored.
3169+
PAG_TEST(PAGXHTMLImporterTest, FoldedImageDataConflictWarnsAndImageWins) {
3170+
auto doc = ParseFromString(R"HTML(
3171+
<html><body style="width:64px;height:64px">
3172+
<div data-id="wrapper" style="width:64px;height:64px;border-radius:9999px;overflow:hidden">
3173+
<img src="avatar.png" data-id="image" style="position:absolute;left:0;top:0;width:64px;height:64px"/>
3174+
</div>
3175+
</body></html>
3176+
)HTML");
3177+
ASSERT_NE(doc, nullptr);
3178+
auto* wrapper = doc->layers.front()->children.front();
3179+
EXPECT_EQ(wrapper->customData["id"], "image");
3180+
bool foundConflict = false;
3181+
for (const auto& error : doc->errors) {
3182+
if (error.find("data-id") != std::string::npos &&
3183+
error.find("overrides") != std::string::npos) {
3184+
foundConflict = true;
3185+
break;
3186+
}
3187+
}
3188+
EXPECT_TRUE(foundConflict);
3189+
}
3190+
31493191
PAG_TEST(PAGXHTMLImporterTest, IdAttributePropagatesToLayer) {
31503192
auto doc = ParseFromString(R"HTML(
31513193
<html><body style="width:50px;height:50px">
@@ -3324,6 +3366,23 @@ PAG_TEST(PAGXHTMLImporterTest, BackgroundImageSizeContainMapsToLetterBox) {
33243366
EXPECT_EQ(pattern->scaleMode, pagx::ScaleMode::LetterBox);
33253367
}
33263368

3369+
// html-snapshot escapes apostrophes inside its single-quoted CSS url(). The importer removes that
3370+
// simple CSS escape so local paths keep their actual filename.
3371+
PAG_TEST(PAGXHTMLImporterTest, BackgroundImageUnescapesQuotedURL) {
3372+
auto doc = ParseFromString(R"HTML(
3373+
<html><body style="width:160px;height:120px">
3374+
<div style="width:160px;height:120px;background-image:url('/tmp/b\'s.png')"></div>
3375+
</body></html>
3376+
)HTML");
3377+
ASSERT_NE(doc, nullptr);
3378+
auto* fill = FindElementOfType<pagx::Fill>(doc->layers.front()->children.front());
3379+
ASSERT_NE(fill, nullptr);
3380+
auto* pattern = As<pagx::ImagePattern>(fill->color);
3381+
ASSERT_NE(pattern, nullptr);
3382+
ASSERT_NE(pattern->image, nullptr);
3383+
EXPECT_EQ(pattern->image->filePath, "/tmp/b's.png");
3384+
}
3385+
33273386
// `background-size: cover` → Zoom.
33283387
PAG_TEST(PAGXHTMLImporterTest, BackgroundImageSizeCoverMapsToZoom) {
33293388
auto doc = ParseFromString(R"HTML(

‎test/src/PAGXOptimizerTest.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,6 +1608,29 @@ CLI_TEST(PAGXOptimizerTest, CollapseDropsShellParent) {
16081608
EXPECT_FLOAT_EQ(child->left, 5.0f);
16091609
}
16101610

1611+
// Bottom-up recursion plus the local fixed-point loop collapses an arbitrarily nested shell chain
1612+
// in one optimizer pass, leaving the painted leaf as the sole top-level layer.
1613+
CLI_TEST(PAGXOptimizerTest, CollapseDropsNestedShellChainInOnePass) {
1614+
auto doc = PAGXDocument::Make(100, 100);
1615+
auto* outer = AddTopLayer(doc.get());
1616+
auto* middle = doc->makeNode<Layer>();
1617+
auto* inner = doc->makeNode<Layer>();
1618+
auto* leaf = doc->makeNode<Layer>();
1619+
leaf->width = 40;
1620+
leaf->height = 20;
1621+
AddRectFill(doc.get(), leaf, 40, 20);
1622+
inner->children.push_back(leaf);
1623+
middle->children.push_back(inner);
1624+
outer->children.push_back(middle);
1625+
1626+
OptimizeWithOptions(doc.get(), CollapseOnly());
1627+
1628+
ASSERT_EQ(doc->layers.size(), 1u);
1629+
EXPECT_EQ(doc->layers.front(), leaf);
1630+
EXPECT_TRUE(leaf->children.empty());
1631+
ASSERT_EQ(leaf->contents.size(), 2u);
1632+
}
1633+
16111634
// A child that applies its own opacity must NOT be absorbed — the alpha would be lost.
16121635
CLI_TEST(PAGXOptimizerTest, CollapseKeepsChildWithAlpha) {
16131636
auto doc = PAGXDocument::Make(100, 100);

0 commit comments

Comments
 (0)