Repository navigation
fix(core): block plugins on custom tags were laid out at 0x0 and never painted - #22
Merged
Merged
Conversation
…r painted
A custom tag (<info-box>, <x-badge>) has no UA display style, so the HTML
adapters build it as an InlineNode. _handleInlineNode only consulted the
inline plugin tags, so a registered BLOCK plugin on such a tag never reached
_tokenizeBlockPlugin: its widget was built, linked to no fragment, sized 0x0
and not painted ('More child widgets than fragments' in debug).
A registered block tag is now handled as a block whatever its node type.
The existing plugin tests hand-built BlockNodes or asserted findsOneWidget,
which a 0x0 widget satisfies. The new tests assert size, position between the
neighbouring paragraphs, painted pixels, stacking, virtualized mode, that the
plugin owns its children, and that unregistered and inline plugins are
unaffected.
Contributor
|
| Fixture | Budget (ms) | Median (ms) | P95 (ms) | |
|---|---|---|---|---|
| ❌ | simple_paragraph |
8 | 17 | 29 |
| ❌ | mixed_inline |
10 | 15 | 18 |
| ❌ | float_layout |
12 | 16 | 25 |
| ❌ | table_20_rows |
14 | 46 | 61 |
| ✅ | cjk_ruby |
14 | 10 | 12 |
| ❌ | large_article |
16 | 40 | 64 |
One or more fixtures exceeded the 16 ms budget.
Flutter
3.41.5· ubuntu-22.04
Informational, does not block merge: CI runners use software
rendering and are 2-3x slower than the devices the budgets are
calibrated for. Compare against the base branch; if a fixture
regressed relative to it, profile with:
flutter test benchmark/layout_regression.dart --reporter expandedand check _performLineLayout / _buildCharacterMapping for
any new O(N²) or O(N log N) paths introduced in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Found by running the demo app for the 1.11.0 release check: the Plugin API demo logged
Layout Warning: More child widgets than fragments. Node: info-boxand (on inspection)<info-box>/<rating>rendered as nothing.A custom tag (
<info-box>,<x-badge>) has no UA display style, so the HTML adapters build it as anInlineNode._handleInlineNodeonly consulted the inline plugin tags, so a registered block plugin on such a tag never reached_tokenizeBlockPlugin: the widget was built, linked to no fragment, laid out at 0x0 and never painted. Only tags that were already block (figure,div) worked — which is what the old tests used.Reproduced on clean
main(probe:SizedBox(height: 40, ColoredBox(amber))as a block plugin on<x-a>→ size 0x0, 0 amber pixels).Fix
A registered block tag is handled as a block whatever its node type (
_handleInlineNodechecks_blockPluginTagsfirst). 9 lines inrender_hyper_box_layout.dart.Why the old tests missed it
They hand-built
BlockNode(tagName: 'figure'), or assertedfindsOneWidget, which a 0x0 widget satisfies. The new tests (test/block_plugin_custom_tag_test.dart) assert size, position between the neighbouring paragraphs, painted pixels, stacking, virtualized mode, that the plugin owns its children, and that unregistered and inline plugins are unaffected.Verification
Not a regression: present on
main/ 1.10.0. Candidate for the next release.