-
Notifications
You must be signed in to change notification settings - Fork 6k
allow flutter tester to disable font loading from asset bundle #33323
allow flutter tester to disable font loading from asset bundle #33323
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
I've added tests that assert that with the flag passed a bundled roboto font is not used (verified this test fails with the flag removed). I also added a test that we can still add new fonts by using the loadFont API |
| DEF_SWITCH(DisableAssetFonts, | ||
| "disable-asset-fonts", | ||
| "Prevents usage of any non-test fonts unless they were explicitly " | ||
| "Loaded via dart:ui font APIs. This option is only available on the " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is it kept only available in the test shell?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh right. hmm. I think should probably ifdef this out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, I guess the thinking is that these switches aren't on an allowlist or surfaced through the embedder, but this one and use-test-fonts are technically still there. We should fix that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh, but if we change a define for switches then we'd have to compile this twice for each host platform
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a debug mode guard
zanderso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Work towards flutter/flutter#103724
TLDR: We want to make the tester able to load assets by passing flutter-asset-dir. Unfortunately that also causes tests to fallback to bundled fonts ahead of Ahem font which is massively breaking to scuba and not necessarily something we want to be easy. Add a flag that prevents registering fonts located in the asset bundle.