Stop the page Content tab from shadowing the content field (#4271) - #4289
Open
wakqasahmed wants to merge 1 commit into
Open
Stop the page Content tab from shadowing the content field (#4271)#4289wakqasahmed wants to merge 1 commit into
wakqasahmed wants to merge 1 commit into
Conversation
getgrav#4271) BlueprintSchema flattens every field into a single map keyed by name, containers included, so the tabs.fields.content tab and the nested markdown content field in pages/default.yaml collided on the same key and the tab (assigned after its own fields are parsed) always won. That left the content field's validate config unreachable, so page body content was never actually validated. Rename the tab's key to content_tab and update modular.yaml and external.yaml, which extend default.yaml and override that same tab, to match. The content field's key is untouched since it is the real page body key referenced throughout Grav and themes. Add a regression test that loads the real pages/default.yaml blueprint and asserts the flattened content item is the markdown field, not the tab, and that its validate config is reachable.
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.
Fixes #4271
In
system/blueprints/pages/default.yaml, the Content tab and the markdown content field are both keyedcontent. BlueprintSchema flattens every field into one map keyed by name, containers included, and it does this by writing the field's own entry after recursing into its children, so the tab's entry gets written last and wins. The markdown field'svalidateconfig sits in that map undercontenttoo, but it's never the one that gets read back, so it never actually runs.I renamed the tab's key to
content_taband left the field's key alone, sincecontentis the actual page body key used throughout Grav and themes and isn't safe to touch.modular.yamlandexternal.yamlboth extenddefault.yamland override that same tab by key, so I updated them tocontent_tabtoo. Otherwise their overrides would stop merging into the tab and would themselves collide with the content field the same way.I checked how much this actually changes at runtime before assuming a rename alone was safe.
checkSafety()(the XSS scan) already runs on this field today regardless of which rule wins, since it only skips fields typedunset, so no new XSS behavior gets switched on here. Type validation is the part that starts working, and the field already hasmax: 0set from #3643 to opt out of the length cap, so in practice this now only requires content to be a string, which is what a textarea always posts anyway.Added a test that loads the real
pages/default.yamlblueprint and asserts the flattenedcontentitem resolves to the markdown field's type and validate config rather than the tab's.Ran the existing blueprint/data tests plus the full page test suite locally, all green, and confirmed the new test fails against the old blueprint (resolves to type
tab) and passes with the fix.