From 772742cbbb4f15060145cb5aac84b158f1c9ffc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Thu, 2 Jul 2026 00:06:02 -0300 Subject: [PATCH] Fix percentage in font-size for UIWebView. Unload fonts from scene in UIWebView. --- ...iwebview_document_scene_layout_refactor.md | 11 +- .agent/plans/uiwebview_document_scene_plan.md | 6 +- include/eepp/ui/uiscenenode.hpp | 2 + src/eepp/ui/uinode.cpp | 43 ++++--- src/eepp/ui/uiscenenode.cpp | 20 ++- src/eepp/ui/uiwebview.cpp | 1 + .../unit_tests/uicss_inheritance_tests.cpp | 2 +- src/tests/unit_tests/uiwebview_tests.cpp | 114 +++++++++++++++++- 8 files changed, 162 insertions(+), 37 deletions(-) diff --git a/.agent/plans/uiwebview_document_scene_layout_refactor.md b/.agent/plans/uiwebview_document_scene_layout_refactor.md index 2b1961c6a..bc457c6d1 100644 --- a/.agent/plans/uiwebview_document_scene_layout_refactor.md +++ b/.agent/plans/uiwebview_document_scene_layout_refactor.md @@ -168,7 +168,11 @@ application UISceneNode `UISceneNode::overFind()` compatibility override. `UIRoot` keeps its layout/self-hit bounds viewport-sized, but embedded document scenes can ask it to traverse child hit testing through the measured document extent. -- Basic author `@font-face` isolation is implemented and covered by UIWebView tests. +- Author `@font-face` isolation and cleanup are implemented. Scene-local aliases resolve before + global font fallback; WebView navigation clears the document scene's previous author aliases and + internally registered font resources; document scene destruction removes any remaining scene-owned + author fonts. Tests cover sibling-scene isolation, navigation cleanup, and WebView destruction + cleanup. - Tests cover the new topology, viewport-vs-extent behavior, scrolling, two-scene style isolation, navigation supersession, and a resize metric regression that guards against no-op queued viewport churn rebuilding RichText. They also cover document @@ -176,9 +180,6 @@ application UISceneNode ### Pending / Follow-Up -- **Author `@font-face` cleanup audit** should verify navigation/destruction cleanup - for scene-local aliases and loaded font resources. The basic scene-local isolation - path is implemented and tested. - **Subresource lifetime coverage** should be completed for every async path described in Phase 6, including deferred CSS, fonts, images, redirects, cookies, and destruction. - **Example and documentation integration** should be completed after the code shape @@ -385,7 +386,7 @@ Steps: 1. Add a scene-local font-face alias registry keyed by CSS family, style, and weight. 2. Register loaded author fonts under scene-unique internal names. 3. Resolve author font aliases before global `FontManager` fallback. -4. Add `clearAuthorFontFaces()` and call it during navigation. +4. Add `clearFontFaces()` and call it during navigation. 5. Remove only this scene's internally registered author fonts during scene destruction. 6. Mark document extent dirty after a font load can affect metrics. diff --git a/.agent/plans/uiwebview_document_scene_plan.md b/.agent/plans/uiwebview_document_scene_plan.md index ea1ddcaf9..0340b3f0d 100644 --- a/.agent/plans/uiwebview_document_scene_plan.md +++ b/.agent/plans/uiwebview_document_scene_plan.md @@ -2,8 +2,8 @@ > Status: IMPLEMENTED WITH FOLLOW-UPS - the owned document scene, real scroll-target > layout widget, viewport/extent split, root-scoped hit-test traversal, and focused -> UIWebView coverage are implemented. Remaining work is cleanup/audit coverage for -> async subresources, examples/docs, and fixed/sticky acceptance tests. +> UIWebView coverage are implemented. Remaining work is broader async subresource +> coverage, examples/docs, and fixed/sticky acceptance tests. ## Goal @@ -453,7 +453,7 @@ Steps: 3. Register author fonts under an internal scene-unique resource name if `FontManager` registration remains required, while preserving the author-visible family only in the scene-local alias. 4. Keep generic/system fonts and explicitly shared application defaults as global fallbacks. -5. Add an explicit `clearDocumentFontFaces()` / `clearAuthorFontFaces()` operation used during +5. Add an explicit `clearFontFaces()` operation used during navigation before new document CSS is loaded. It removes only this scene's internally registered author fonts and clears aliases; it must not remove application/system fonts or sibling-document author fonts. diff --git a/include/eepp/ui/uiscenenode.hpp b/include/eepp/ui/uiscenenode.hpp index 7ab4a499b..ea1bf5d97 100644 --- a/include/eepp/ui/uiscenenode.hpp +++ b/include/eepp/ui/uiscenenode.hpp @@ -822,6 +822,8 @@ class EE_API UISceneNode : public SceneNode { Font* getFontFromNamesList( std::string_view names, Uint32 fontStyle = 0, FontWeight weight = FontWeight::Normal ) const; + void clearFontFaces(); + Font* reevaluateFontStyle( Font* currentFont, Uint32 fontStyle, FontWeight weight = FontWeight::Normal ) const; diff --git a/src/eepp/ui/uinode.cpp b/src/eepp/ui/uinode.cpp index 3bd9b6d13..156d601e1 100644 --- a/src/eepp/ui/uinode.cpp +++ b/src/eepp/ui/uinode.cpp @@ -1702,9 +1702,29 @@ Float UINode::lengthFromValue( const StyleSheetProperty& property, if ( property.getPropertyDefinition() && property.getPropertyDefinition()->getPropertyId() == PropertyId::FontSize ) { StyleSheetLength length( property.value() ); + auto parentFontSize = [this]() { + Float fontSize = 12.f * PixelDensity::getPixelDensity(); + Node* parentNode = getParent(); + while ( parentNode ) { + if ( parentNode->isWidget() ) { + fontSize = getAbsoluteFontSize( parentNode->asType() ); + break; + } + parentNode = parentNode->getParent(); + } + return fontSize; + }; + auto resolveFontRelativeLength = [this, &parentFontSize]( const StyleSheetLength& len ) { + const Float parentSize = parentFontSize(); + Font* font = nullptr; + if ( getUISceneNode() && getUISceneNode()->getUIThemeManager() ) + font = getUISceneNode()->getUIThemeManager()->getDefaultFont(); + return len.asPixels( parentSize, Sizef::Zero, getSceneNode()->getDPI(), parentSize, + parentSize, font ); + }; + if ( length.getUnit() == StyleSheetLength::Unit::Percentage ) { - length.setValue( length.getValue() / 100.f, StyleSheetLength::Unit::Em ); - return convertLength( length, 0 ); + return resolveFontRelativeLength( length ); } static constexpr std::string_view FontSizeNames[] = { @@ -1738,6 +1758,8 @@ Float UINode::lengthFromValue( const StyleSheetProperty& property, } else if ( keyword == "larger" ) { res.setValue( 1.2f, StyleSheetLength::Unit::Em ); } + if ( res.getUnit() == StyleSheetLength::Unit::Em ) + return resolveFontRelativeLength( res ); return convertLength( res, 0 ); } else if ( property.getValue() == "inherit" ) { Node* parentNode = getParent(); @@ -1757,22 +1779,7 @@ Float UINode::lengthFromValue( const StyleSheetProperty& property, length.getUnit() != StyleSheetLength::Unit::Ch ) return convertLength( length, 0 ); - Float parentFontSize = 12.f * PixelDensity::getPixelDensity(); - Node* parentNode = getParent(); - while ( parentNode ) { - if ( parentNode->isWidget() ) { - parentFontSize = getAbsoluteFontSize( parentNode->asType() ); - break; - } - parentNode = parentNode->getParent(); - } - - Font* font = nullptr; - if ( getUISceneNode() && getUISceneNode()->getUIThemeManager() ) - font = getUISceneNode()->getUIThemeManager()->getDefaultFont(); - - return length.asPixels( 0, Sizef::Zero, getSceneNode()->getDPI(), parentFontSize, - parentFontSize, font ); + return resolveFontRelativeLength( length ); } return lengthFromValue( property.getValue(), property.getPropertyDefinition()->getRelativeTarget(), defaultValue, diff --git a/src/eepp/ui/uiscenenode.cpp b/src/eepp/ui/uiscenenode.cpp index 44fc300dd..6025ef6d1 100644 --- a/src/eepp/ui/uiscenenode.cpp +++ b/src/eepp/ui/uiscenenode.cpp @@ -111,13 +111,11 @@ UISceneNode::~UISceneNode() { mAsyncResourceLoadState->generation++; } + clearFontFaces(); + eeSAFE_DELETE( mUIThemeManager ); eeSAFE_DELETE( mUIIconThemeManager ); - for ( auto& font : mFontFaces ) { - FontManager::instance()->remove( font ); - } - // UISceneNode can now destroy the ThreadPool shared to him. If that's the case, // We need to ensure that the children are destroyed before the thread pool, // since its children could be consuming it and need to uninitialize gracefully. @@ -1937,6 +1935,20 @@ Font* UISceneNode::getFontFromNamesList( std::string_view names, Uint32 fontStyl return font; } +void UISceneNode::clearFontFaces() { + if ( mFontFaces.empty() && mFontFaceAliases.empty() ) + return; + + mFontFaceAliases.clear(); + if ( mRoot ) + mRoot->reloadFontFamily(); + + for ( auto& font : mFontFaces ) + FontManager::instance()->remove( font ); + + mFontFaces.clear(); +} + Font* UISceneNode::reevaluateFontStyle( Font* currentFont, Uint32 fontStyle, FontWeight weight ) const { if ( !currentFont || !SystemFontResolver::isEnabled() ) diff --git a/src/eepp/ui/uiwebview.cpp b/src/eepp/ui/uiwebview.cpp index 14237ab13..09274e9ba 100644 --- a/src/eepp/ui/uiwebview.cpp +++ b/src/eepp/ui/uiwebview.cpp @@ -375,6 +375,7 @@ void UIWebView::loadDocumentData( URI url, std::string data, Uint64 generation ) self->getHorizontalScrollBar()->setValue( 0 ); static_cast( self->mDocContainer )->clearDocumentChildren(); ui->invalidateAsyncResourceLoads(); + ui->clearFontFaces(); ui->getStyleSheet().removeAllWithoutMarker( self->mStyleSheetDefaultMarker ); ui->setURIFromURL( url ); diff --git a/src/tests/unit_tests/uicss_inheritance_tests.cpp b/src/tests/unit_tests/uicss_inheritance_tests.cpp index b8a9f3ff8..be7bd4f05 100644 --- a/src/tests/unit_tests/uicss_inheritance_tests.cpp +++ b/src/tests/unit_tests/uicss_inheritance_tests.cpp @@ -153,7 +153,7 @@ UTEST( CSSInheritance, ComputedFontSizePercentageAndRem ) { UIWidget* targetSpan = root->querySelector( "#targetspan" ); EXPECT_TRUE( targetSpan != nullptr ); - EXPECT_NEAR( 18u * scale, targetSpan->asType()->getFontSize(), 1.f ); + EXPECT_NEAR( 30u * scale, targetSpan->asType()->getFontSize(), 1.f ); } } diff --git a/src/tests/unit_tests/uiwebview_tests.cpp b/src/tests/unit_tests/uiwebview_tests.cpp index 48d70292b..10fef21cc 100644 --- a/src/tests/unit_tests/uiwebview_tests.cpp +++ b/src/tests/unit_tests/uiwebview_tests.cpp @@ -203,10 +203,12 @@ UTEST( UIWebView, FontSizeEmDoesNotCompoundOnViewportRelayout ) { html, body { margin: 0; padding: 0; } body { font-size: 16px; } h1 { font-size: 2.5em; margin: 0.5em 0; } + h2 { font-size: 250%; margin: 0.5em 0; } -

Title

+

Title em

+

Title percent

@@ -224,16 +226,20 @@ UTEST( UIWebView, FontSizeEmDoesNotCompoundOnViewportRelayout ) { }; pump(); - Node* title = documentScene->getRoot()->find( "title-text" ); - ASSERT_TRUE( title != nullptr && title->isType( UI_TYPE_TEXTSPAN ) ); - EXPECT_NEAR( title->asType()->getFontSize(), 40.f, 1.f ); + Node* titleEm = documentScene->getRoot()->find( "title-em" ); + Node* titlePercent = documentScene->getRoot()->find( "title-percent" ); + ASSERT_TRUE( titleEm != nullptr && titleEm->isType( UI_TYPE_TEXTSPAN ) ); + ASSERT_TRUE( titlePercent != nullptr && titlePercent->isType( UI_TYPE_TEXTSPAN ) ); + EXPECT_NEAR( titleEm->asType()->getFontSize(), 40.f, 1.f ); + EXPECT_NEAR( titlePercent->asType()->getFontSize(), 40.f, 1.f ); for ( int i = 0; i < 4; i++ ) { webView->setPixelsSize( 520 + i * 20, 320 + i * 10 ); pump(); webView->setPixelsSize( 420, 260 ); pump(); - EXPECT_NEAR( title->asType()->getFontSize(), 40.f, 1.f ); + EXPECT_NEAR( titleEm->asType()->getFontSize(), 40.f, 1.f ); + EXPECT_NEAR( titlePercent->asType()->getFontSize(), 40.f, 1.f ); } Engine::destroySingleton(); @@ -1479,6 +1485,8 @@ UTEST( UIWebView, DocumentScenesIsolateAuthorFontFaces ) { const std::string processPath( Sys::getProcessPath() ); const std::string pathA = Sys::getTempPath() + "eepp_uiwebview_font_doc_a.html"; const std::string pathB = Sys::getTempPath() + "eepp_uiwebview_font_doc_b.html"; + const std::string pathAWithoutFont = + Sys::getTempPath() + "eepp_uiwebview_font_doc_a_without_font.html"; const std::string pathA2 = Sys::getTempPath() + "eepp_uiwebview_font_doc_a2.html"; FileSystem::fileWrite( pathA, "B" ); + FileSystem::fileWrite( pathAWithoutFont, + "A empty" + "" ); FileSystem::fileWrite( pathA2, "A" ); + FileSystem::fileWrite( pathB, + "B" ); + + webViewA->loadURI( URI( "file://" + pathA ) ); + webViewB->loadURI( URI( "file://" + pathB ) ); + + auto pump = [&]() { + for ( int i = 0; i < 10; i++ ) { + win->getInput()->update(); + SceneManager::instance()->update( Seconds( 1.f / 60.f ) ); + } + }; + pump(); + + UISceneNode* docA = webViewA->getDocumentSceneNode(); + UISceneNode* docB = webViewB->getDocumentSceneNode(); + ASSERT_TRUE( docA != nullptr ); + ASSERT_TRUE( docB != nullptr ); + Font* loadedFontA = docA->getFontFromNamesList( "DestroyDocFace" ); + Font* loadedFontB = docB->getFontFromNamesList( "DestroyDocFace" ); + ASSERT_TRUE( loadedFontA != nullptr ); + ASSERT_TRUE( loadedFontB != nullptr ); + EXPECT_TRUE( loadedFontA->loaded() ); + EXPECT_TRUE( loadedFontB->loaded() ); + EXPECT_NE( loadedFontA, loadedFontB ); + const std::string loadedFontAName = loadedFontA->getName(); + const std::string loadedFontBName = loadedFontB->getName(); + EXPECT_EQ( loadedFontA, FontManager::instance()->getByName( loadedFontAName ) ); + EXPECT_EQ( loadedFontB, FontManager::instance()->getByName( loadedFontBName ) ); + + webViewA->close(); + pump(); + + EXPECT_EQ( nullptr, FontManager::instance()->getByName( loadedFontAName ) ); + EXPECT_EQ( loadedFontB, FontManager::instance()->getByName( loadedFontBName ) ); + EXPECT_EQ( loadedFontB, docB->getFontFromNamesList( "DestroyDocFace" ) ); + + Engine::destroySingleton(); +} + UTEST( UIWebView, NewerNavigationSupersedesStartedLoad ) { auto win = Engine::instance()->createWindow( WindowSettings( 800, 600, "UIWebView Stale Navigation Test", WindowStyle::Default,