Html api rem attr sc flag#87
Draft
sirreal wants to merge 18 commits into
Draft
Conversation
…ng flags.
Removing an attribute that is preceded by a solidus ("/") can leave the
solidus directly before the tag-closing ">", where it becomes a
self-closing flag and changes the meaning of the surrounding HTML:
<svg><g /attr>ok → remove "attr" → <svg><g />ok
In foreign content the G element no longer contains the following text.
See #65372.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Solidus characters ("/") may precede an attribute name, where they act
like attribute separators and are not part of any syntax token. Removing
an attribute without its preceding solidus characters could leave a
solidus directly before the tag-closing ">", where it becomes a
self-closing flag and changes the meaning of the surrounding HTML:
<svg><g /attr>ok → remove "attr" → <svg><g />ok
Extend attribute removal spans backward over any solidus characters
directly preceding the attribute so the separators are removed with
the attribute:
<svg><g /attr>ok → remove "attr" → <svg><g >ok
See #65372.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
…offset.
When a removed attribute is separated from the tag name by only solidus
characters, its removal span starts at the end of the tag name, the same
document offset where new attributes are inserted. The lexical update
application loop assumes update spans never overlap and moves its cursor
backward in this case, un-doing the removal:
<g/attr>ok → set_attribute( 'id', 'test' ) + remove_attribute( 'attr' )
→ <g id="test"/attr>ok
See #65372.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
The lexical update application loop assumed that update spans never share a document offset. Attribute removal spans extended over their preceding solidus separators may start at the end of the tag name, where new attributes are inserted. When both updates were enqueued, the cursor moved backward after the zero-length insertion applied, copying the removed attribute back into the document. Skip copying when an update starts at or before the copied-bytes cursor and never move the cursor backward. Updates which previously moved the cursor backward produced corrupted output, so this changes no valid behavior. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
… bookmarks.
Removing an attribute also enqueues removals for its duplicates under
numeric keys. Repeating the removal enqueues the duplicate removals
again. Bookmark positions and the internal cursor are shifted by every
enqueued update when updates are applied, so the repeated updates shift
them more than the document actually changed:
<div a a>ok<path id="x"> → remove "a" twice, flush, seek to PATH
→ lands on the "ok" text, get_tag() is null
The updated HTML appears correct; only positional accounting breaks.
This also affects trunk, where the repeated updates additionally
corrupt the updated HTML itself.
See #65372.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Removing an attribute also enqueues removals for its duplicates under numeric keys, so repeated removals of the same attribute enqueued the duplicate removals again. Bookmark positions and the internal cursor are shifted by every enqueued update when updates are applied; the repeated updates shifted them more than the document actually changed, corrupting bookmarks and seek() destinations. Skip enqueuing the duplicate removals when they are already enqueued. Duplicate removals are enqueued as a batch: if the first one is present, all of them are. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
The guard against repeatedly enqueuing duplicate attribute removals matches enqueued updates by their span alone. An update enqueued over the same span with different replacement text is not a removal of that span, but it suppresses the duplicate removal batch, leaving later duplicates in the document. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
…als. The guard against repeatedly enqueuing duplicate attribute removals matched enqueued updates by their span alone, so an update enqueued over the same span with different replacement text suppressed the duplicate removal batch, leaving later duplicates in the document. Require empty replacement text: only a removal of the first duplicate's span indicates that the batch is already enqueued. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Cover both operation orders when an attribute is added while a solidus-separated attribute is removed, and verify bookmark positions after the shared-offset updates are applied. Verify that a genuine self-closing flag in foreign content survives attribute removal along with the resulting document structure. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
State the invariants the update application loop relies on: updates with positive length must never overlap, only zero-length insertions may share an offset with another update's span, and same-offset ordering places insertions after a removal of the span they share. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
…te spans. Detecting an already-enqueued batch of duplicate attribute removals by the first duplicate's span alone fails in two ways when another lexical update targets that span: - An enqueued removal of only the first duplicate's span suppresses the batch, leaving later duplicates in the document. - Any other update over the span coexists with the batch, so two updates replace the same span and bookmark positions shift more than the document actually changed. Exercise both cases on the same processor with exact expected HTML and a bookmark past the edits. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Detecting an already-enqueued batch of duplicate attribute removals by the first duplicate's span alone fails when another lexical update targets that span: an enqueued removal of only the first duplicate's span suppressed the whole batch, and any other update over a duplicate's span coexisted with the batch, replacing the same span twice and shifting bookmark positions more than the document changed. Verify each duplicate's span individually and enqueue only missing removals. An update already enqueued over a duplicate's span is superseded by the removal, so no two updates replace overlapping spans of the document. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
State on the protected lexical updates property that code creating updates must ensure no two updates replace overlapping spans of the document, since cursor and bookmark position accounting assumes every update replaces a distinct span. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Attribute removal spans extend over preceding solidus characters, so an update enqueued over the attribute's original token span intersects the removal span without matching it exactly. The exact-span supersede misses it and enqueues a removal replacing an overlapping span: - A replacement within the span survives in the output. - A removal within the span double-counts the removed bytes, so bookmark positions shift more than the document changed and seek() lands off the bookmarked tag. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Attribute removal spans extend over preceding solidus characters, so detecting an already-enqueued removal by exact span match misses updates enqueued over spans within the removal span, such as the attribute's original token span. The removal was then enqueued alongside them, and two updates replaced overlapping spans of the document: replaced text could survive in the output, and bookmark positions shifted more than the document changed. Remove every enqueued update replacing a span which intersects the removal span before enqueuing the removal: removal replaces the entire span, and exactly one update accounts for the removed bytes. This applies to the attribute's own span and to its duplicates' spans, and it also keeps repeated removals of the same attribute idempotent. See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
Superseding enqueued updates separately for the attribute's span and for each duplicate's span scanned the update queue once per span while each removal grew the queue, making removal of a duplicated attribute quadratic in the number of duplicates: removing an attribute duplicated 10,000 times in a 20 KB tag took hundreds of times longer than parsing the document. Collect the removal spans first and supersede intersecting updates in one sweep over the queue. Attribute spans appear in document order and do not overlap, so each enqueued update is checked against the sorted spans with a single binary search: an update can only intersect the last removal span starting before the update's end. Removing an attribute duplicated 20,000 times now costs less than parsing the document (2.7 s before, 2.4 ms after). See #65372. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BZSpa317PhG11sX3YUDcGU
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
1 participant
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.
Trac ticket:
Use of AI Tools
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.