Skip to content

Conversation

@sammygill
Copy link
Contributor

@sammygill sammygill commented Nov 7, 2024

84a4a18

thesession.org: [content-visibility] SVG text not shown in content-visibility: auto subtree.
https://bugs.webkit.org/show_bug.cgi?id=281570
rdar://138040315

Reviewed by Alan Baradlay.

RenderSVGText expects to be able to perform some sort of initialization on its first pass
of layout by checking everHadLayout(). Unfortunately, if this renderer is in a
content-visibility: auto subtree, the content-visibility logic will descend down that
subtree, clearing layout on all the renderers, which ends up setting the "ever had layout,"
bit. This ends up confusing RenderSVGText since it thinks it has gone through layout and
assumes that it had performed this initialization, which can result in us not computing
the proper geometry for the content.

To address this, let's keep track of a bit on RenderSVGText that indicates if it has ever
performed a full pass of layout. We can use this in the same places we were previously
using everHadLayout().

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-auto-svg-text-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-auto-svg-text.html: Added.
* Source/WebCore/rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::shouldHandleSubtreeMutations const):
(WebCore::RenderSVGText::subtreeTextDidChange):
(WebCore::RenderSVGText::layout):
* Source/WebCore/rendering/svg/RenderSVGText.h:

Canonical link: https://commits.webkit.org/286447@main

4354e13

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@sammygill sammygill self-assigned this Nov 7, 2024
@sammygill sammygill changed the title wip thesession.org: [content-visibility] SVG text not shown in content-visibility: auto subtree. Nov 8, 2024
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from 7fb5942 to 36256da Compare November 8, 2024 21:19
@sammygill sammygill added the CSS Cascading Style Sheets implementation label Nov 8, 2024
@sammygill sammygill marked this pull request as ready for review November 8, 2024 21:19
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from 36256da to c415157 Compare November 8, 2024 21:21
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from c415157 to b2bd93d Compare November 8, 2024 21:27
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from b2bd93d to 6610903 Compare November 8, 2024 22:49
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from 6610903 to 5c590ce Compare November 8, 2024 22:51
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 9, 2024
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Nov 11, 2024
@sammygill sammygill force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from 5c590ce to 4354e13 Compare November 11, 2024 17:24
@sammygill sammygill added the merge-queue Applied to send a pull request to merge-queue label Nov 11, 2024
…sibility: auto subtree.

https://bugs.webkit.org/show_bug.cgi?id=281570
rdar://138040315

Reviewed by Alan Baradlay.

RenderSVGText expects to be able to perform some sort of initialization on its first pass
of layout by checking everHadLayout(). Unfortunately, if this renderer is in a
content-visibility: auto subtree, the content-visibility logic will descend down that
subtree, clearing layout on all the renderers, which ends up setting the "ever had layout,"
bit. This ends up confusing RenderSVGText since it thinks it has gone through layout and
assumes that it had performed this initialization, which can result in us not computing
the proper geometry for the content.

To address this, let's keep track of a bit on RenderSVGText that indicates if it has ever
performed a full pass of layout. We can use this in the same places we were previously
using everHadLayout().

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-auto-svg-text-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-auto-svg-text.html: Added.
* Source/WebCore/rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::shouldHandleSubtreeMutations const):
(WebCore::RenderSVGText::subtreeTextDidChange):
(WebCore::RenderSVGText::layout):
* Source/WebCore/rendering/svg/RenderSVGText.h:

Canonical link: https://commits.webkit.org/286447@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/content-visibility-content-visibility-auto-prevents-text-nodes-in-SVG-from-being-painted branch from 4354e13 to 84a4a18 Compare November 11, 2024 20:47
@webkit-commit-queue
Copy link
Collaborator

Committed 286447@main (84a4a18): https://commits.webkit.org/286447@main

Reviewed commits have been landed. Closing PR #36342 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 84a4a18 into WebKit:main Nov 11, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants