fix: Fix img layout issue / support CSS display:none for elements and images (#1443)
## Summary - Add CSS `display: none` support to the EPUB rendering pipeline (fixes #1431) - Parse `display` property in stylesheets and inline styles, with full cascade resolution (element, class, element.class, inline) - Skip hidden elements and all their descendants in `ChapterHtmlSlimParser` - Separate display:none check for `<img>` tags (image code path is independent of the general element handler) - Flush pending text blocks before placing images to fix layout ordering (text preceding an image now correctly renders above it) - Bump CSS cache version to 4 to invalidate stale caches - Add test EPUB (`test_display_none.epub`) covering class selectors, element selectors, combined selectors, inline styles, nested hidden content, hidden images, style priority/override, and realistic use cases
This commit is contained in:
@@ -52,6 +52,29 @@ constexpr size_t MAX_SELECTOR_LENGTH = 256;
|
|||||||
// Check if character is CSS whitespace
|
// Check if character is CSS whitespace
|
||||||
bool isCssWhitespace(const char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f'; }
|
bool isCssWhitespace(const char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f'; }
|
||||||
|
|
||||||
|
std::string_view stripTrailingImportant(std::string_view value) {
|
||||||
|
constexpr std::string_view IMPORTANT = "!important";
|
||||||
|
|
||||||
|
while (!value.empty() && isCssWhitespace(value.back())) {
|
||||||
|
value.remove_suffix(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (value.size() < IMPORTANT.size()) {
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
|
const size_t suffixPos = value.size() - IMPORTANT.size();
|
||||||
|
if (value.substr(suffixPos) != IMPORTANT) {
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
|
value.remove_suffix(IMPORTANT.size());
|
||||||
|
while (!value.empty() && isCssWhitespace(value.back())) {
|
||||||
|
value.remove_suffix(1);
|
||||||
|
}
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
} // anonymous namespace
|
} // anonymous namespace
|
||||||
|
|
||||||
// String utilities implementation
|
// String utilities implementation
|
||||||
@@ -317,6 +340,10 @@ void CssParser::parseDeclarationIntoStyle(const std::string& decl, CssStyle& sty
|
|||||||
style.imageWidth = len;
|
style.imageWidth = len;
|
||||||
style.defined.imageWidth = 1;
|
style.defined.imageWidth = 1;
|
||||||
}
|
}
|
||||||
|
} else if (propNameBuf == "display") {
|
||||||
|
const std::string_view displayValue = stripTrailingImportant(propValueBuf);
|
||||||
|
style.display = (displayValue == "none") ? CssDisplay::None : CssDisplay::Block;
|
||||||
|
style.defined.display = 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -692,6 +719,7 @@ bool CssParser::saveToCache() const {
|
|||||||
writeLength(style.paddingRight);
|
writeLength(style.paddingRight);
|
||||||
writeLength(style.imageHeight);
|
writeLength(style.imageHeight);
|
||||||
writeLength(style.imageWidth);
|
writeLength(style.imageWidth);
|
||||||
|
file.write(static_cast<uint8_t>(style.display));
|
||||||
|
|
||||||
// Write defined flags as uint16_t
|
// Write defined flags as uint16_t
|
||||||
uint16_t definedBits = 0;
|
uint16_t definedBits = 0;
|
||||||
@@ -710,6 +738,7 @@ bool CssParser::saveToCache() const {
|
|||||||
if (style.defined.paddingRight) definedBits |= 1 << 12;
|
if (style.defined.paddingRight) definedBits |= 1 << 12;
|
||||||
if (style.defined.imageHeight) definedBits |= 1 << 13;
|
if (style.defined.imageHeight) definedBits |= 1 << 13;
|
||||||
if (style.defined.imageWidth) definedBits |= 1 << 14;
|
if (style.defined.imageWidth) definedBits |= 1 << 14;
|
||||||
|
if (style.defined.display) definedBits |= 1 << 15;
|
||||||
file.write(reinterpret_cast<const uint8_t*>(&definedBits), sizeof(definedBits));
|
file.write(reinterpret_cast<const uint8_t*>(&definedBits), sizeof(definedBits));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -748,16 +777,44 @@ bool CssParser::loadFromCache() {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (ruleCount > MAX_RULES) {
|
||||||
|
LOG_DBG("CSS", "Invalid cache rule count (%u > %zu)", ruleCount, MAX_RULES);
|
||||||
|
rulesBySelector_.clear();
|
||||||
|
file.close();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
auto hasRemainingBytes = [&file](const size_t neededBytes) -> bool {
|
||||||
|
return static_cast<size_t>(file.available()) >= neededBytes;
|
||||||
|
};
|
||||||
|
|
||||||
|
constexpr size_t CSS_LENGTH_FIELD_COUNT = 11;
|
||||||
|
constexpr size_t CSS_LENGTH_BYTES = sizeof(float) + sizeof(uint8_t);
|
||||||
|
constexpr size_t CSS_FIXED_STYLE_BYTES =
|
||||||
|
4 * sizeof(uint8_t) + (CSS_LENGTH_FIELD_COUNT * CSS_LENGTH_BYTES) + sizeof(uint8_t) + sizeof(uint16_t);
|
||||||
|
|
||||||
// Read each rule
|
// Read each rule
|
||||||
for (uint16_t i = 0; i < ruleCount; ++i) {
|
for (uint16_t i = 0; i < ruleCount; ++i) {
|
||||||
// Read selector string
|
// Read selector string
|
||||||
uint16_t selectorLen = 0;
|
uint16_t selectorLen = 0;
|
||||||
|
if (!hasRemainingBytes(sizeof(selectorLen))) {
|
||||||
|
rulesBySelector_.clear();
|
||||||
|
file.close();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
if (file.read(&selectorLen, sizeof(selectorLen)) != sizeof(selectorLen)) {
|
if (file.read(&selectorLen, sizeof(selectorLen)) != sizeof(selectorLen)) {
|
||||||
rulesBySelector_.clear();
|
rulesBySelector_.clear();
|
||||||
file.close();
|
file.close();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (selectorLen == 0 || selectorLen > MAX_SELECTOR_LENGTH || !hasRemainingBytes(selectorLen)) {
|
||||||
|
LOG_DBG("CSS", "Invalid selector length in cache: %u", selectorLen);
|
||||||
|
rulesBySelector_.clear();
|
||||||
|
file.close();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
std::string selector;
|
std::string selector;
|
||||||
selector.resize(selectorLen);
|
selector.resize(selectorLen);
|
||||||
if (file.read(&selector[0], selectorLen) != selectorLen) {
|
if (file.read(&selector[0], selectorLen) != selectorLen) {
|
||||||
@@ -766,6 +823,13 @@ bool CssParser::loadFromCache() {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!hasRemainingBytes(CSS_FIXED_STYLE_BYTES)) {
|
||||||
|
LOG_DBG("CSS", "Truncated CSS cache while reading style payload");
|
||||||
|
rulesBySelector_.clear();
|
||||||
|
file.close();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
// Read CssStyle fields
|
// Read CssStyle fields
|
||||||
CssStyle style;
|
CssStyle style;
|
||||||
uint8_t enumVal;
|
uint8_t enumVal;
|
||||||
@@ -820,6 +884,15 @@ bool CssParser::loadFromCache() {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Read display value
|
||||||
|
uint8_t displayVal;
|
||||||
|
if (file.read(&displayVal, 1) != 1) {
|
||||||
|
rulesBySelector_.clear();
|
||||||
|
file.close();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
style.display = static_cast<CssDisplay>(displayVal);
|
||||||
|
|
||||||
// Read defined flags
|
// Read defined flags
|
||||||
uint16_t definedBits = 0;
|
uint16_t definedBits = 0;
|
||||||
if (file.read(&definedBits, sizeof(definedBits)) != sizeof(definedBits)) {
|
if (file.read(&definedBits, sizeof(definedBits)) != sizeof(definedBits)) {
|
||||||
@@ -842,6 +915,7 @@ bool CssParser::loadFromCache() {
|
|||||||
style.defined.paddingRight = (definedBits & 1 << 12) != 0;
|
style.defined.paddingRight = (definedBits & 1 << 12) != 0;
|
||||||
style.defined.imageHeight = (definedBits & 1 << 13) != 0;
|
style.defined.imageHeight = (definedBits & 1 << 13) != 0;
|
||||||
style.defined.imageWidth = (definedBits & 1 << 14) != 0;
|
style.defined.imageWidth = (definedBits & 1 << 14) != 0;
|
||||||
|
style.defined.display = (definedBits & 1 << 15) != 0;
|
||||||
|
|
||||||
rulesBySelector_[selector] = style;
|
rulesBySelector_[selector] = style;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,7 +31,7 @@
|
|||||||
class CssParser {
|
class CssParser {
|
||||||
public:
|
public:
|
||||||
// Bump when CSS cache format or rules change; section caches are invalidated when this changes
|
// Bump when CSS cache format or rules change; section caches are invalidated when this changes
|
||||||
static constexpr uint8_t CSS_CACHE_VERSION = 3;
|
static constexpr uint8_t CSS_CACHE_VERSION = 4;
|
||||||
|
|
||||||
explicit CssParser(std::string cachePath) : cachePath(std::move(cachePath)) {}
|
explicit CssParser(std::string cachePath) : cachePath(std::move(cachePath)) {}
|
||||||
~CssParser() = default;
|
~CssParser() = default;
|
||||||
|
|||||||
@@ -54,6 +54,9 @@ enum class CssFontWeight : uint8_t { Normal = 0, Bold = 1 };
|
|||||||
// Text decoration options
|
// Text decoration options
|
||||||
enum class CssTextDecoration : uint8_t { None = 0, Underline = 1 };
|
enum class CssTextDecoration : uint8_t { None = 0, Underline = 1 };
|
||||||
|
|
||||||
|
// Display options - only None and Block are relevant for e-ink rendering
|
||||||
|
enum class CssDisplay : uint8_t { Block = 0, None = 1 };
|
||||||
|
|
||||||
// Bitmask for tracking which properties have been explicitly set
|
// Bitmask for tracking which properties have been explicitly set
|
||||||
struct CssPropertyFlags {
|
struct CssPropertyFlags {
|
||||||
uint16_t textAlign : 1;
|
uint16_t textAlign : 1;
|
||||||
@@ -71,6 +74,7 @@ struct CssPropertyFlags {
|
|||||||
uint16_t paddingRight : 1;
|
uint16_t paddingRight : 1;
|
||||||
uint16_t imageHeight : 1;
|
uint16_t imageHeight : 1;
|
||||||
uint16_t imageWidth : 1;
|
uint16_t imageWidth : 1;
|
||||||
|
uint16_t display : 1;
|
||||||
|
|
||||||
CssPropertyFlags()
|
CssPropertyFlags()
|
||||||
: textAlign(0),
|
: textAlign(0),
|
||||||
@@ -87,19 +91,20 @@ struct CssPropertyFlags {
|
|||||||
paddingLeft(0),
|
paddingLeft(0),
|
||||||
paddingRight(0),
|
paddingRight(0),
|
||||||
imageHeight(0),
|
imageHeight(0),
|
||||||
imageWidth(0) {}
|
imageWidth(0),
|
||||||
|
display(0) {}
|
||||||
|
|
||||||
[[nodiscard]] bool anySet() const {
|
[[nodiscard]] bool anySet() const {
|
||||||
return textAlign || fontStyle || fontWeight || textDecoration || textIndent || marginTop || marginBottom ||
|
return textAlign || fontStyle || fontWeight || textDecoration || textIndent || marginTop || marginBottom ||
|
||||||
marginLeft || marginRight || paddingTop || paddingBottom || paddingLeft || paddingRight || imageHeight ||
|
marginLeft || marginRight || paddingTop || paddingBottom || paddingLeft || paddingRight || imageHeight ||
|
||||||
imageWidth;
|
imageWidth || display;
|
||||||
}
|
}
|
||||||
|
|
||||||
void clearAll() {
|
void clearAll() {
|
||||||
textAlign = fontStyle = fontWeight = textDecoration = textIndent = 0;
|
textAlign = fontStyle = fontWeight = textDecoration = textIndent = 0;
|
||||||
marginTop = marginBottom = marginLeft = marginRight = 0;
|
marginTop = marginBottom = marginLeft = marginRight = 0;
|
||||||
paddingTop = paddingBottom = paddingLeft = paddingRight = 0;
|
paddingTop = paddingBottom = paddingLeft = paddingRight = 0;
|
||||||
imageHeight = imageWidth = 0;
|
imageHeight = imageWidth = display = 0;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -123,6 +128,7 @@ struct CssStyle {
|
|||||||
CssLength paddingRight; // Padding right
|
CssLength paddingRight; // Padding right
|
||||||
CssLength imageHeight; // Height for img (e.g. 2em) – width derived from aspect ratio when only height set
|
CssLength imageHeight; // Height for img (e.g. 2em) – width derived from aspect ratio when only height set
|
||||||
CssLength imageWidth; // Width for img when both or only width set
|
CssLength imageWidth; // Width for img when both or only width set
|
||||||
|
CssDisplay display = CssDisplay::Block; // display property (Block or None)
|
||||||
|
|
||||||
CssPropertyFlags defined; // Tracks which properties were explicitly set
|
CssPropertyFlags defined; // Tracks which properties were explicitly set
|
||||||
|
|
||||||
@@ -189,6 +195,10 @@ struct CssStyle {
|
|||||||
imageWidth = base.imageWidth;
|
imageWidth = base.imageWidth;
|
||||||
defined.imageWidth = 1;
|
defined.imageWidth = 1;
|
||||||
}
|
}
|
||||||
|
if (base.hasDisplay()) {
|
||||||
|
display = base.display;
|
||||||
|
defined.display = 1;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
[[nodiscard]] bool hasTextAlign() const { return defined.textAlign; }
|
[[nodiscard]] bool hasTextAlign() const { return defined.textAlign; }
|
||||||
@@ -206,6 +216,7 @@ struct CssStyle {
|
|||||||
[[nodiscard]] bool hasPaddingRight() const { return defined.paddingRight; }
|
[[nodiscard]] bool hasPaddingRight() const { return defined.paddingRight; }
|
||||||
[[nodiscard]] bool hasImageHeight() const { return defined.imageHeight; }
|
[[nodiscard]] bool hasImageHeight() const { return defined.imageHeight; }
|
||||||
[[nodiscard]] bool hasImageWidth() const { return defined.imageWidth; }
|
[[nodiscard]] bool hasImageWidth() const { return defined.imageWidth; }
|
||||||
|
[[nodiscard]] bool hasDisplay() const { return defined.display; }
|
||||||
|
|
||||||
void reset() {
|
void reset() {
|
||||||
textAlign = CssTextAlign::Left;
|
textAlign = CssTextAlign::Left;
|
||||||
@@ -216,6 +227,7 @@ struct CssStyle {
|
|||||||
marginTop = marginBottom = marginLeft = marginRight = CssLength{};
|
marginTop = marginBottom = marginLeft = marginRight = CssLength{};
|
||||||
paddingTop = paddingBottom = paddingLeft = paddingRight = CssLength{};
|
paddingTop = paddingBottom = paddingLeft = paddingRight = CssLength{};
|
||||||
imageHeight = imageWidth = CssLength{};
|
imageHeight = imageWidth = CssLength{};
|
||||||
|
display = CssDisplay::Block;
|
||||||
defined.clearAll();
|
defined.clearAll();
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -182,6 +182,24 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
|
|||||||
centeredBlockStyle.textAlignDefined = true;
|
centeredBlockStyle.textAlignDefined = true;
|
||||||
centeredBlockStyle.alignment = CssTextAlign::Center;
|
centeredBlockStyle.alignment = CssTextAlign::Center;
|
||||||
|
|
||||||
|
// Compute CSS style for this element early so display:none can short-circuit
|
||||||
|
// before tag-specific branches emit any content or metadata.
|
||||||
|
CssStyle cssStyle;
|
||||||
|
if (self->cssParser) {
|
||||||
|
cssStyle = self->cssParser->resolveStyle(name, classAttr);
|
||||||
|
if (!styleAttr.empty()) {
|
||||||
|
CssStyle inlineStyle = CssParser::parseInlineStyle(styleAttr);
|
||||||
|
cssStyle.applyOver(inlineStyle);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip elements with display:none before all fast paths (tables, links, etc.).
|
||||||
|
if (cssStyle.hasDisplay() && cssStyle.display == CssDisplay::None) {
|
||||||
|
self->skipUntilDepth = self->depth;
|
||||||
|
self->depth += 1;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Special handling for tables/cells: flatten into per-cell paragraphs with a prefixed header.
|
// Special handling for tables/cells: flatten into per-cell paragraphs with a prefixed header.
|
||||||
if (strcmp(name, "table") == 0) {
|
if (strcmp(name, "table") == 0) {
|
||||||
// skip nested tables
|
// skip nested tables
|
||||||
@@ -264,6 +282,19 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Skip image if CSS display:none
|
||||||
|
if (self->cssParser) {
|
||||||
|
CssStyle imgDisplayStyle = self->cssParser->resolveStyle("img", classAttr);
|
||||||
|
if (!styleAttr.empty()) {
|
||||||
|
imgDisplayStyle.applyOver(CssParser::parseInlineStyle(styleAttr));
|
||||||
|
}
|
||||||
|
if (imgDisplayStyle.hasDisplay() && imgDisplayStyle.display == CssDisplay::None) {
|
||||||
|
self->skipUntilDepth = self->depth;
|
||||||
|
self->depth += 1;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (!src.empty() && self->imageRendering != 1) {
|
if (!src.empty() && self->imageRendering != 1) {
|
||||||
LOG_DBG("EHP", "Found image: src=%s", src.c_str());
|
LOG_DBG("EHP", "Found image: src=%s", src.c_str());
|
||||||
|
|
||||||
@@ -384,6 +415,15 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
|
|||||||
LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale);
|
LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Flush any pending text block so it appears before the image
|
||||||
|
if (self->partWordBufferIndex > 0) {
|
||||||
|
self->flushPartWordBuffer();
|
||||||
|
}
|
||||||
|
if (self->currentTextBlock && !self->currentTextBlock->isEmpty()) {
|
||||||
|
const BlockStyle parentBlockStyle = self->currentTextBlock->getBlockStyle();
|
||||||
|
self->startNewTextBlock(parentBlockStyle);
|
||||||
|
}
|
||||||
|
|
||||||
// Create page for image - only break if image won't fit remaining space
|
// Create page for image - only break if image won't fit remaining space
|
||||||
if (self->currentPage && !self->currentPage->elements.empty() &&
|
if (self->currentPage && !self->currentPage->elements.empty() &&
|
||||||
(self->currentPageNextY + displayHeight > self->viewportHeight)) {
|
(self->currentPageNextY + displayHeight > self->viewportHeight)) {
|
||||||
@@ -514,18 +554,6 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char*
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Compute CSS style for this element
|
|
||||||
CssStyle cssStyle;
|
|
||||||
if (self->cssParser) {
|
|
||||||
// Get combined tag + class styles
|
|
||||||
cssStyle = self->cssParser->resolveStyle(name, classAttr);
|
|
||||||
// Merge inline style (highest priority)
|
|
||||||
if (!styleAttr.empty()) {
|
|
||||||
CssStyle inlineStyle = CssParser::parseInlineStyle(styleAttr);
|
|
||||||
cssStyle.applyOver(inlineStyle);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const float emSize = static_cast<float>(self->renderer.getFontAscenderSize(self->fontId));
|
const float emSize = static_cast<float>(self->renderer.getFontAscenderSize(self->fontId));
|
||||||
const auto userAlignmentBlockStyle = BlockStyle::fromCssStyle(
|
const auto userAlignmentBlockStyle = BlockStyle::fromCssStyle(
|
||||||
cssStyle, emSize, static_cast<CssTextAlign>(self->paragraphAlignment), self->viewportWidth);
|
cssStyle, emSize, static_cast<CssTextAlign>(self->paragraphAlignment), self->viewportWidth);
|
||||||
|
|||||||
BIN
test/epubs/test_display_none.epub
Normal file
BIN
test/epubs/test_display_none.epub
Normal file
Binary file not shown.
Reference in New Issue
Block a user