diff --git a/include/eepp/ui/uihtmltable.hpp b/include/eepp/ui/uihtmltable.hpp index 7a060eb4d..8b4735bff 100644 --- a/include/eepp/ui/uihtmltable.hpp +++ b/include/eepp/ui/uihtmltable.hpp @@ -6,6 +6,9 @@ namespace EE { namespace UI { +class UIHTMLTableRow; +class UIHTMLTableCell; + class EE_API UIHTMLTable : public UILayout { public: static UIHTMLTable* New(); @@ -20,6 +23,11 @@ class EE_API UIHTMLTable : public UILayout { protected: virtual Uint32 onMessage( const NodeMessage* Msg ); + + std::vector mRows; + std::vector mColWidths; + std::vector mCells; + std::vector mRowCellOffsets; }; class EE_API UIHTMLTableCell : public UIRichText { diff --git a/src/eepp/ui/tools/htmlformatter.cpp b/src/eepp/ui/tools/htmlformatter.cpp index 1e2738dd3..338b688c2 100644 --- a/src/eepp/ui/tools/htmlformatter.cpp +++ b/src/eepp/ui/tools/htmlformatter.cpp @@ -175,22 +175,24 @@ String HTMLFormatter::collapseXmlWhitespace( const String& text, const pugi::xml } } - // Step 2: Determine if the left boundary is a block element. + // Step 2: Determine if the left boundary is a block element or a forced line break (
). // We use getLogicalPrev, and if the previous node is just empty space // (lacks significant text), we keep looking further back. pugi::xml_node prev = getLogicalPrev( node ); while ( prev && prev.type() == pugi::node_pcdata && !hasSignificantText( prev ) ) { prev = getLogicalPrev( prev ); } - bool prevInline = isInlineNode( prev ); + // A node is a valid inline neighbor only if it is an inline node AND not a
+ // (because
strips adjacent whitespace). + bool prevInline = isInlineNode( prev ) && !String::iequals( prev.name(), "br" ); - // Step 3: Determine if the right boundary is a block element. + // Step 3: Determine if the right boundary is a block element or a forced line break. // We use getLogicalNext, skipping over any non-significant text nodes. pugi::xml_node next = getLogicalNext( node ); while ( next && next.type() == pugi::node_pcdata && !hasSignificantText( next ) ) { next = getLogicalNext( next ); } - bool nextInline = isInlineNode( next ); + bool nextInline = isInlineNode( next ) && !String::iequals( next.name(), "br" ); // Step 4: Trim leading and trailing spaces if they adjoin a block boundary. if ( !prevInline && !res.empty() && res[0] == ' ' ) diff --git a/src/eepp/ui/uihtmltable.cpp b/src/eepp/ui/uihtmltable.cpp index 8ed07bc40..b619ed67c 100644 --- a/src/eepp/ui/uihtmltable.cpp +++ b/src/eepp/ui/uihtmltable.cpp @@ -25,17 +25,15 @@ void UIHTMLTable::updateLayout() { return; mPacking = true; - // TODO: Optimize this horrendous implementation (fix the heap-allocation crazyness) UIHTMLTableHead* head = nullptr; UIHTMLTableBody* body = nullptr; UIHTMLTableFooter* footer = nullptr; - std::vector rows; - std::function collectRows = [&]( Node* node ) { - Node* child = node->getFirstChild(); - while ( child ) { + mRows.clear(); + auto collectRows = [&]( auto self, Node* node ) -> void { + for ( Node* child = node->getFirstChild(); child; child = child->getNextNode() ) { if ( child->getType() == UI_TYPE_HTML_TABLE_ROW ) { - rows.push_back( child->asType() ); + mRows.push_back( child->asType() ); } else if ( child->getType() != UI_TYPE_HTML_TABLE ) { if ( child->getType() == UI_TYPE_HTML_TABLE_HEAD ) head = child->asType(); @@ -44,30 +42,31 @@ void UIHTMLTable::updateLayout() { else if ( child->getType() == UI_TYPE_HTML_TABLE_FOOTER ) footer = child->asType(); - collectRows( child ); + self( self, child ); } - child = child->getNextNode(); } }; - collectRows( this ); + collectRows( collectRows, this ); - if ( rows.empty() ) { + if ( mRows.empty() ) { mPacking = false; return; } - std::vector> grid; + mCells.clear(); + mRowCellOffsets.clear(); + mRowCellOffsets.push_back( 0 ); size_t maxCols = 0; - for ( auto* row : rows ) { - std::vector cells; - Node* child = row->getFirstChild(); - while ( child ) { - if ( child->getType() == UI_TYPE_HTML_TABLE_CELL ) - cells.push_back( child->asType() ); - child = child->getNextNode(); + for ( auto* row : mRows ) { + size_t cellCount = 0; + for ( Node* child = row->getFirstChild(); child; child = child->getNextNode() ) { + if ( child->getType() == UI_TYPE_HTML_TABLE_CELL ) { + mCells.push_back( child->asType() ); + cellCount++; + } } - grid.push_back( cells ); - maxCols = std::max( maxCols, cells.size() ); + mRowCellOffsets.push_back( (Uint32)mCells.size() ); + maxCols = std::max( maxCols, cellCount ); } if ( maxCols == 0 ) { @@ -75,31 +74,29 @@ void UIHTMLTable::updateLayout() { return; } - std::vector colWidths( maxCols, 0.f ); + mColWidths.assign( maxCols, 0.f ); // Get natural width for each column (without wrapping) - for ( const auto& rowCells : grid ) { - for ( size_t i = 0; i < rowCells.size(); ++i ) { - UIHTMLTableCell* cell = rowCells[i]; + for ( size_t r = 0; r < mRows.size(); ++r ) { + Uint32 start = mRowCellOffsets[r]; + Uint32 end = mRowCellOffsets[r + 1]; + for ( Uint32 i = 0; i < end - start; ++i ) { + UIHTMLTableCell* cell = mCells[start + i]; cell->setLayoutWidthPolicy( SizePolicy::WrapContent ); cell->updateLayout(); - colWidths[i] = std::max( colWidths[i], cell->getPixelsSize().getWidth() ); + mColWidths[i] = std::max( mColWidths[i], cell->getPixelsSize().getWidth() ); } } Float availableWidth = getPixelsSize().getWidth() - mPaddingPx.Left - mPaddingPx.Right; Float totalUnwrappedWidth = 0; - for ( Float w : colWidths ) + for ( Float w : mColWidths ) totalUnwrappedWidth += w; - if ( totalUnwrappedWidth > availableWidth && totalUnwrappedWidth > 0 ) { + if ( totalUnwrappedWidth > 0 ) { Float scale = availableWidth / totalUnwrappedWidth; for ( size_t i = 0; i < maxCols; ++i ) - colWidths[i] *= scale; - } else if ( totalUnwrappedWidth < availableWidth && maxCols > 0 && totalUnwrappedWidth > 0 ) { - Float scale = availableWidth / totalUnwrappedWidth; - for ( size_t i = 0; i < maxCols; ++i ) - colWidths[i] *= scale; + mColWidths[i] *= scale; } Float headHeight = 0; @@ -107,35 +104,37 @@ void UIHTMLTable::updateLayout() { Float footerHeight = 0; // Apply layout and calculate heights - size_t rowCount = grid.size(); + size_t rowCount = mRows.size(); for ( size_t r = 0; r < rowCount; ++r ) { Float rowHeight = 0; - size_t columnCount = grid[r].size(); - for ( size_t c = 0; c < columnCount; ++c ) { - UIHTMLTableCell* cell = grid[r][c]; + Uint32 start = mRowCellOffsets[r]; + Uint32 end = mRowCellOffsets[r + 1]; + Uint32 columnCount = end - start; + for ( Uint32 c = 0; c < columnCount; ++c ) { + UIHTMLTableCell* cell = mCells[start + c]; cell->setLayoutWidthPolicy( SizePolicy::Fixed ); - cell->setPixelsSize( colWidths[c], cell->getPixelsSize().getHeight() ); + cell->setPixelsSize( mColWidths[c], cell->getPixelsSize().getHeight() ); cell->updateLayout(); rowHeight = std::max( rowHeight, cell->getPixelsSize().getHeight() ); } // Position cells inside the row and equalize height Float currentX = 0; - for ( size_t c = 0; c < columnCount; ++c ) { - UIHTMLTableCell* cell = grid[r][c]; + for ( Uint32 c = 0; c < columnCount; ++c ) { + UIHTMLTableCell* cell = mCells[start + c]; cell->setPixelsPosition( currentX, 0 ); cell->setPixelsSize( cell->getPixelsSize().getWidth(), rowHeight ); - currentX += colWidths[c]; + currentX += mColWidths[c]; } // Set row height and width - UIHTMLTableRow* row = rows[r]; + UIHTMLTableRow* row = mRows[r]; row->setPixelsSize( availableWidth, rowHeight ); if ( r == 0 ) { headHeight = rowHeight; } else if ( r == rowCount - 1 && columnCount && - grid[r][0]->getParent()->isType( UI_TYPE_HTML_TABLE_FOOTER ) ) { + mCells[start]->getParent()->isType( UI_TYPE_HTML_TABLE_FOOTER ) ) { footerHeight = rowHeight; } else { bodyHeight += rowHeight; @@ -143,8 +142,8 @@ void UIHTMLTable::updateLayout() { } // Position rows vertically - // We also need to ensure that the containers (thead, tbody, etc.) are positioned at 0,0 - // and have the correct size, so the absolute positioning of the rows works as expected. + // We also need to ensure that the containers (thead, tbody, etc.) are positioned correctly and + // have the correct size, so the absolute positioning of the rows works as expected. if ( head ) { head->setPixelsPosition( 0, 0 ); head->setPixelsSize( { getPixelsSize().x, headHeight } ); @@ -161,15 +160,18 @@ void UIHTMLTable::updateLayout() { } Float currentY = mPaddingPx.Top - headHeight; - for ( auto* row : rows ) { + for ( size_t r = 0; r < rowCount; ++r ) { + UIHTMLTableRow* row = mRows[r]; row->setPixelsPosition( mPaddingPx.Left, currentY ); currentY += row->getPixelsSize().getHeight(); } - if ( head && !rows.empty() ) - rows[0]->setPixelsPosition( mPaddingPx.Left, 0 ); - if ( footer && !rows.empty() ) - rows[rowCount - 1]->setPixelsPosition( mPaddingPx.Left, 0 ); + // Reset positions if they are inside specialized containers + if ( head && !mRows.empty() ) + mRows[0]->setPixelsPosition( mPaddingPx.Left, 0 ); + + if ( footer && !mRows.empty() ) + mRows[rowCount - 1]->setPixelsPosition( mPaddingPx.Left, 0 ); if ( mWidthPolicy == SizePolicy::MatchParent ) setInternalPixelsWidth( getMatchParentWidth() ); diff --git a/src/tests/unit_tests/richtext.cpp b/src/tests/unit_tests/richtext.cpp index 5bed370e1..1803e389c 100644 --- a/src/tests/unit_tests/richtext.cpp +++ b/src/tests/unit_tests/richtext.cpp @@ -836,3 +836,55 @@ UTEST( UIHTMLTable, basicLayout ) { eeDelete( sceneNode ); Engine::destroySingleton(); } + +UTEST( UIRichText, WhitespaceCollapseBRTest ) { + Engine::instance()->createWindow( WindowSettings( 800, 600, "RichText Test", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ) ); + FileSystem::changeWorkingDirectory( Sys::getProcessPath() ); + + FontTrueType* font = FontTrueType::New( "NotoSans-Regular" ); + font->loadFromFile( "../assets/fonts/NotoSans-Regular.ttf" ); + + ASSERT_TRUE( font->loaded() ); + FontFamily::loadFromRegular( font ); + + UI::UISceneNode* sceneNode = UI::UISceneNode::New(); + UI::UIThemeManager* themeManager = sceneNode->getUIThemeManager(); + themeManager->setDefaultFont( font ); + + String xml = R"xml( +

+
+ ecode +

+ )xml"; + + sceneNode->loadLayoutFromString( xml ); + + UI::UIRichText* rt = sceneNode->find( "rt" ); + ASSERT_TRUE( rt != nullptr ); + + sceneNode->update( Time::Zero ); + + // The "ecode" text span should NOT have a leading space. + bool foundEcodeWithLeadingSpace = false; + auto checkSpansRecursive = [&]( Node* n, auto&& checkSpansRecursiveRef ) -> void { + if ( !n ) return; + if ( n->isWidget() && n->isType( UI_TYPE_TEXTSPAN ) ) { + UI::UITextSpan* span = static_cast( n ); + if ( span->getText().size() > 0 && span->getText()[0] == ' ' && + span->getText().find( "ecode" ) != String::InvalidPos ) { + foundEcodeWithLeadingSpace = true; + } + } + for ( Node* child = n->getFirstChild(); child; child = child->getNextNode() ) { + checkSpansRecursiveRef( child, checkSpansRecursiveRef ); + } + }; + checkSpansRecursive( rt, checkSpansRecursive ); + EXPECT_FALSE( foundEcodeWithLeadingSpace ); + + eeDelete( sceneNode ); + Engine::destroySingleton(); +}