mirror of
https://github.com/webstudio-is/webstudio.git
synced 2026-10-23 03:27:27 +09:00
fix: Accept description-list groups wrapped in divs (#6346)
Pasting `<dl><div><dt>Company</dt><dd>XXX</dd></div></dl>` incorrectly warns that placing `dt` inside `div` violates HTML. The [HTML standard](https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element) allows these groups. The shared validator now tracks the rendered HTML parent explicitly and gives a `div` directly under `dl` the description-group content model. Parent context passes through Slots and Fragments and works when validation starts within the list. Ordinary divs and extra HTML wrappers still reject `dt`/`dd`; group divs reject unrelated flow children. No synthetic category can be mistaken for an allowed HTML tag. - [x] Fix contextual validation without changing generated HTML data. - [x] Add nine regression cases with before/after failure verification. - [x] Run 170 tests covering content models, matching, components, and pre-publish audits. - [x] Pass project-build typecheck, repository lint, formatting, and diff checks. - [x] Review the patch and fix the internal-marker validation bypass. - [x] Review documentation impact: no update needed; this restores valid HTML support. - [x] Review fixture impact: no description-list fixture inputs or generated outputs are affected. - [x] Confirm CI passes on the final commit before merging: all workflows passed on `4ce4cf6696`, including all six browser-test shards. This remains a nesting check; enforcing term/definition order or counts is outside this fix. Agent evaluations and local browser E2E were not run; CI runs the browser suites. Closes #6345
This commit is contained in:
1 parent
e33ce76489
commit
d991b25b65
2 files changed
+174
-12
No files matched your search
@@ -152,6 +152,149 @@ test("none category accepted by parent by tag", () => {
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
||||
describe("description lists", () => {
|
||||
test("does not accept an internal context marker as an HTML tag", () => {
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<ws.element ws:tag="dl-child" />
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["listId"],
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test.each([
|
||||
["listId"],
|
||||
["groupId", "listId"],
|
||||
["termId", "groupId", "listId"],
|
||||
["detailId", "groupId", "listId"],
|
||||
])("accepts div groups when validating from %j", (...instanceSelector) => {
|
||||
const errors: string[] = [];
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<ws.element ws:tag="div" ws:id="groupId">
|
||||
<ws.element ws:tag="dt" ws:id="termId">
|
||||
Company
|
||||
</ws.element>
|
||||
<ws.element ws:tag="dd" ws:id="detailId">
|
||||
XXX
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
<ws.element ws:tag="div">
|
||||
<ws.element ws:tag="dt">Tax ID (NIP EU)</ws.element>
|
||||
<ws.element ws:tag="dd">XXX</ws.element>
|
||||
</ws.element>
|
||||
<ws.element ws:tag="div">
|
||||
<ws.element ws:tag="dt">Address</ws.element>
|
||||
<ws.element ws:tag="dd">...</ws.element>
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector,
|
||||
onError: (message) => errors.push(message),
|
||||
})
|
||||
).toBe(true);
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
test("accepts div groups through components without HTML tags", () => {
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<Fragment>
|
||||
<ws.element ws:tag="div">
|
||||
<Slot>
|
||||
<ws.element ws:tag="dt">Company</ws.element>
|
||||
<ws.element ws:tag="dd">XXX</ws.element>
|
||||
</Slot>
|
||||
</ws.element>
|
||||
</Fragment>
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["listId"],
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects flow content directly inside a description-list group", () => {
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<ws.element ws:tag="div">
|
||||
<ws.element ws:tag="p">Company</ws.element>
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["listId"],
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test.each(["dt", "dd"])("keeps %s restricted to description lists", (tag) => {
|
||||
for (const [ancestor, parent] of [
|
||||
["div", "div"],
|
||||
["dl", "div"],
|
||||
["dl", "a"],
|
||||
["dl", "section"],
|
||||
]) {
|
||||
const data = renderData(
|
||||
<ws.element ws:tag={ancestor} ws:id="rootId">
|
||||
<ws.element ws:tag={parent} ws:id="groupId">
|
||||
<ws.element ws:tag="div" ws:id="wrapperId">
|
||||
<ws.element ws:tag={tag} ws:id="childId">
|
||||
XXX
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
);
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...data,
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["childId", "wrapperId", "groupId", "rootId"],
|
||||
})
|
||||
).toBe(false);
|
||||
}
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<ws.element ws:tag="div">
|
||||
<ws.element ws:tag={tag}>XXX</ws.element>
|
||||
</ws.element>
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["listId"],
|
||||
})
|
||||
).toBe(true);
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
...renderData(
|
||||
<ws.element ws:tag="dl" ws:id="listId">
|
||||
<ws.element ws:tag="dt">Company</ws.element>
|
||||
<ws.element ws:tag="dd">XXX</ws.element>
|
||||
</ws.element>
|
||||
),
|
||||
metas: defaultMetas,
|
||||
instanceSelector: ["listId"],
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test("none category prevents unacceptable parent", () => {
|
||||
expect(
|
||||
isTreeSatisfyingContentModel({
|
||||
|
||||
@@ -118,7 +118,8 @@ const isTagSatisfyingContentModel = ({
|
||||
*/
|
||||
const getElementChildren = (
|
||||
tag: undefined | string,
|
||||
allowedCategories: undefined | string[]
|
||||
allowedCategories: undefined | string[],
|
||||
parentTag: undefined | string
|
||||
) => {
|
||||
// A transparent component without a known parent imposes no constraint.
|
||||
if (tag === undefined && allowedCategories === undefined) {
|
||||
@@ -137,6 +138,10 @@ const getElementChildren = (
|
||||
category === "transparent" ? inheritedCategories : category
|
||||
);
|
||||
}
|
||||
// A div directly under dl contains name-value groups instead of flow content.
|
||||
if (tag === "div" && parentTag === "dl") {
|
||||
elementChildren = ["dt", "dd", "script-supporting elements"];
|
||||
}
|
||||
// introduce custom non-interactive category to restrict nesting interactive elements
|
||||
// like button > button or a > input
|
||||
if (
|
||||
@@ -186,6 +191,7 @@ const computeAllowedCategories = ({
|
||||
}) => {
|
||||
let instance: undefined | Instance;
|
||||
let allowedCategories: undefined | string[];
|
||||
let parentTag: undefined | string;
|
||||
// skip selected instance for which these constraints are computed
|
||||
for (const instanceId of instanceSelector.slice(1).reverse()) {
|
||||
instance = instances.get(instanceId);
|
||||
@@ -194,9 +200,11 @@ const computeAllowedCategories = ({
|
||||
continue;
|
||||
}
|
||||
const tag = getTag({ instance, metas, props, htmlTagsByInstanceId });
|
||||
allowedCategories = getElementChildren(tag, allowedCategories);
|
||||
allowedCategories = getElementChildren(tag, allowedCategories, parentTag);
|
||||
// Tagless components do not change the rendered HTML parent.
|
||||
parentTag = tag ?? parentTag;
|
||||
}
|
||||
return allowedCategories;
|
||||
return { allowedCategories, parentTag };
|
||||
};
|
||||
|
||||
const findHtmlConstraintInstance = ({
|
||||
@@ -217,6 +225,7 @@ const findHtmlConstraintInstance = ({
|
||||
component: Instance["component"];
|
||||
}) => {
|
||||
let allowedCategories: undefined | string[];
|
||||
let parentTag: undefined | string;
|
||||
let wasSatisfying = true;
|
||||
let constraintInstance: undefined | Instance;
|
||||
|
||||
@@ -231,7 +240,12 @@ const findHtmlConstraintInstance = ({
|
||||
props,
|
||||
htmlTagsByInstanceId,
|
||||
});
|
||||
allowedCategories = getElementChildren(ancestorTag, allowedCategories);
|
||||
allowedCategories = getElementChildren(
|
||||
ancestorTag,
|
||||
allowedCategories,
|
||||
parentTag
|
||||
);
|
||||
parentTag = ancestorTag ?? parentTag;
|
||||
const isSatisfying = isTagSatisfyingContentModel({
|
||||
tag,
|
||||
component,
|
||||
@@ -417,6 +431,7 @@ export const isTreeSatisfyingContentModel = ({
|
||||
htmlTagsByInstanceId = getHtmlTagsFromProps(props),
|
||||
onError,
|
||||
_allowedCategories: allowedCategories,
|
||||
_parentTag: parentTag,
|
||||
_allowedAncestorCategories: allowedAncestorCategories,
|
||||
_allowedParentCategories: allowedParentCategories,
|
||||
}: {
|
||||
@@ -427,17 +442,20 @@ export const isTreeSatisfyingContentModel = ({
|
||||
htmlTagsByInstanceId?: HtmlTagsByInstanceId;
|
||||
onError?: (message: string, instanceSelector: InstanceSelector) => void;
|
||||
_allowedCategories?: string[];
|
||||
_parentTag?: string;
|
||||
_allowedAncestorCategories?: string[];
|
||||
_allowedParentCategories?: string[];
|
||||
}): boolean => {
|
||||
// compute constraints only when not passed from parent
|
||||
allowedCategories ??= computeAllowedCategories({
|
||||
instanceSelector,
|
||||
instances,
|
||||
props,
|
||||
metas,
|
||||
htmlTagsByInstanceId,
|
||||
});
|
||||
if (allowedCategories === undefined) {
|
||||
({ allowedCategories, parentTag } = computeAllowedCategories({
|
||||
instanceSelector,
|
||||
instances,
|
||||
props,
|
||||
metas,
|
||||
htmlTagsByInstanceId,
|
||||
}));
|
||||
}
|
||||
allowedParentCategories ??= getAllowedParentCategories({
|
||||
instanceSelector,
|
||||
instances,
|
||||
@@ -535,7 +553,7 @@ export const isTreeSatisfyingContentModel = ({
|
||||
isSatisfying = false;
|
||||
}
|
||||
const contentModel = getComponentContentModel(metas.get(instance.component));
|
||||
allowedCategories = getElementChildren(tag, allowedCategories);
|
||||
allowedCategories = getElementChildren(tag, allowedCategories, parentTag);
|
||||
allowedParentCategories = contentModel.children;
|
||||
if (contentModel.descendants) {
|
||||
allowedAncestorCategories ??= [];
|
||||
@@ -554,6 +572,7 @@ export const isTreeSatisfyingContentModel = ({
|
||||
instanceSelector: [child.value, ...instanceSelector],
|
||||
onError,
|
||||
_allowedCategories: allowedCategories,
|
||||
_parentTag: tag ?? parentTag,
|
||||
_allowedParentCategories: allowedParentCategories,
|
||||
_allowedAncestorCategories: allowedAncestorCategories,
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user