Rescue: <select> support found unpushed in a temp directory - #96
Closed
pathscale wants to merge 18 commits into
Closed
Rescue: <select> support found unpushed in a temp directory#96pathscale wants to merge 18 commits into
pathscale wants to merge 18 commits into
Conversation
added 18 commits
September 9, 2026 19:38
The type list that grants the 300px-by-line-height intrinsic content box omitted "number", while the list in layout::construct that decides which inputs get a text editor included it. A number input therefore got an editor and a zero content box, measuring 6x6 (its padding and border) against 306x25.2 for every other text-like type.
…g it flush_styles_to_layout returns early on a display:none subtree, so a stacking-context host hidden after its child list was built never rebuilds that list. resolve_hoisted_positions walks every node that has a context, hidden or not, and indexed the slab directly, so removing one of the listed children panicked with "invalid SlotMap key used" at resolve.rs:863 on the next resolve. Reported 6 times out of 6 from a signup page whose header menu switched language and then opened a chat launcher.
Two defects on the same walk of controls_to_form. The map is never pruned when a control leaves the document, so a framework that re-renders a field leaves a freed id in it. The walk indexed the slab for every id it found, so setting a value twice and then pressing Enter panicked with "invalid SlotMap key used" and took the host down. The walk now goes over the tree, which only yields live nodes, and removal prunes the map so it cannot grow without bound. The "more than one field that blocks implicit submission" rule is gated by the spec on the form having no submit button. It was applied unconditionally, so Enter never submitted any form with two explicitly typed fields even when the form had a Sign in button. Three sites hit this.
Answers a QA finding that reported hover leaving every rendered pixel unchanged on four controls. Neither proposed cause holds in the engine: the hover media feature cannot evaluate false here (make_device passes a fixed PointerCapabilities::default(), with no host input), and a UiEvent::PointerMove followed by a resolve does reach the paint, including when the hover colour arrives through a custom property gated on (hover: hover), which is the shape the component library ships.
…tton The implicit-submission helper had grown a second definition of the button case. Delegate to the existing one, which also handles the command attributes that take a button back out of the Submit Button state, and keep only the input types here.
The legacy alias for the stop-propagation flag, in neither the shim nor the bundled framework, so a delegated dispatcher had nothing to read and stopPropagation did not hold. Measured on two sites: pressing a cookie-preferences category dismissed the entire dialog, because the closing backdrop is an ancestor of the panel. Reading it reports the flag; setting it true raises it, and setting it false does nothing, which is what the DOM standard says.
blitz-dom has had scroll_to_node all along, for the harness's agent action, but page script could not reach it: getElementById(...).scrollIntoView() raised "TypeError: not a callable function". Any anchor-scrolling router, a "back to top" control and a validation-error focuser all call it. The argument is accepted and ignored, because scroll_to_node lands the node at the top-left of each scrollport, which is the default block: "start".
location was a plain data object, so a page calling any of the three threw a TypeError out of whatever ran it. All three now go through the document's navigation provider, which is where a link click already goes; BaseDocument grows navigate_to_url and current_url because both resolve_url and the provider were internal to blitz-dom.
…window dispatchEvent history.back and history.forward changed the URL and announced nothing, so a router's navigate(-1) never redrew and every route was one-way. Announcing it needed a way to raise a window-targeted event at all: Node.prototype.dispatchEvent walks a node chain and never reaches the window listeners, so window.dispatchEvent is added here and the history shim uses it.
…een edited change was synthesised only for checkbox and radio, so any onChange on a text field was dead. That shipped a product bug: a phone-number field bound to change gated a Confirm button that nothing could ever open. A text control commits rather than notifying, so the value at focus is recorded and compared on blur; an untouched field still fires nothing. The markup5ever dev-dependency is for the test, which sets a field's value the way the DOM does.
There was no select rule in the user-agent stylesheet, so a select computed
display: inline, and option { display: none } left it with no in-flow content.
Height on a non-replaced inline is ignored, so even a select with an authored
height reported 0x0 and nothing on a page could find it or press it.
Its content size cannot come from flow, because the options it is sized from
are not in it, so layout measures the widest option label directly. The
character-count estimate is the same one the textarea cols attribute already
uses: a select's label is not laid out as text anywhere yet.
The popup, option hit-testing and selection are NOT implemented. A select now
has a box and can be found and pressed; pressing it still does nothing.
Table layout flattens rows into a CSS grid of cells, so tr, thead and tbody nodes have their box construction damage cleared and never reach Taffy. Nothing ever wrote a layout for them: every one reported 0x0 and, to anything asking whether an element is displayed, "no". A QA check walking a table by row had nothing to walk. A row is not laid out, it is described, so the context now records which cells belong to which row and which rows to which group, and a pass after rounding derives each box from them. After rounding because final_layout is what every geometry query reads and the rounding pass is what fills it; reading the cells any earlier gets zeroes.
…e internals
table { display: block } is the standard wide-table horizontal-scroll pattern,
and it made every row render side by side: the header on one line, then every
data row at the same y and increasing x. With the table no longer a table, its
thead and tbody were plain block children, and their own displays have no
mapping in the style conversion, so they fell through to Taffy's default, which
is flex, and a flex container lays its items out in a row.
CSS 2.1 17.2.1 requires an anonymous table box around misparented
table-internal boxes. When the container holds nothing else, which is this
whole pattern and every case seen on a real page, that anonymous box would be
its only child and take its content box, so it is fused into the container
instead of inserted under it. A container with mixed content still needs a
separate anonymous box and does not get one here.
The QA fixes in this branch are what downstream needs, and nothing can ask for them until they are released. The branch originally carried a bump to 0.4.2; master reached that version independently and has since released 0.4.6, so the same intent is now a bump to 0.4.7. Thirteen commits sit above the 0.4.6 release commit and none of them are on crates.io: 0fbbf52 size input[type=number] like the other text inputs 01b484e prune removed nodes from a stacking context before resolving it 594d298 make implicit form submission safe and spec-correct 3bcf5f8 reuse ElementData::is_submit_button for the default button afcb61e expose Event.cancelBubble 2995467 expose Element.scrollIntoView bddcd96 give window.location assign, replace and reload 44de0c7 dispatch popstate on a history traversal, and give the window dispatchEvent e405cc3 fire change on a text control that loses focus having been edited fc4eee8 give <select> a layout box 1e40949 give table rows and row groups the box their cells occupy c6c185c generate a table box for a block whose children are table internals Two of them close SlotMap panics that took the host process down: a freed control id left in controls_to_form, and a freed child left in a stacking context. The patch, not the minor: every change is a fix or an addition. The new public surface is BaseDocument::navigate_to_url and current_url, plus Element scrollIntoView, Event.cancelBubble, window.dispatchEvent and location assign, replace and reload on the script side. Nothing public is removed or narrowed: the table context gained fields, but blitz_dom::layout is a private module. The in-repo ranges stay at ^0.4.0, which accepts 0.4.7, so the whole bump is one line.
… tree There was no notion of selectedness anywhere in the engine. An <option> was a display:none node carrying a `selected` attribute nobody read, so a select could be measured and pressed but could not say what it offered or what was chosen. Its options reached the accessibility tree with a role and nothing else: no label, no selected state, and no value on the select itself. A QA harness driving worktables.dev's schema designer had nothing to assert against. SpecialElementData::Select(SelectData) holds one selectedness flag per option, seeded at layout construction from the `selected` content attributes and from HTML's "ask for a reset" step, which is why a plain <select> now reports its first option rather than an empty value. Construction is idempotent the way create_checkbox_input is, so a selection survives the next resolve; only the option count is refreshed, which is how a script-added option gets an entry. Positions rather than node ids: selectedIndex is the spec's own handle and a position cannot dangle once the option is removed. The list of options is recomputed from the subtree on every read, which flattens <optgroup> for free.
…bmittable Three ways in, none of which worked. `handle_click` had no arm for a select, so the walk fell through to the no-match tail, which calls `clear_focus()`. Pressing a select actively unfocused the page, and the keyboard handler is gated on focus, so the arrows could not drive a control the user had just pressed. `checkable_activation_target` gets the matching arm, because the two have to agree about where the walk ends. Form submission had a standing TODO exactly where a select's serialization belongs, so a select fell through to the generic tail and submitted its own literal `value` attribute, which a select does not have. Every form containing a picker posted the wrong body. HTMLSelectElement existed only as an instanceof brand. `select.value` read an attribute that is never there and reported the empty string; there was no selectedIndex, no options, and no option.selected. Those go on the shared Element prototype behind a tag guard, the way `value` already branches on text_input_data(), and they write live state then snapshot, which is the rule set_checked's comment lays down.
…en it does There was no keyboard activation for anything but a text input, so a focussed picker could not be driven at all. The arrows never even reached the keyboard handler: the KeyDown default action scrolls the page on ArrowUp, ArrowDown, Home and End and then returns, and `scroll_key_is_claimed_by`, the one way past it, tested only for a text input. A press on a select scrolled the document instead of choosing anything. ArrowUp and ArrowDown step over disabled options and do not wrap; Home and End go to the first and last selectable option. A selection that does not move dispatches nothing, so a `change` listener does not fire on a no-op. The keystroke is the commit, so `input` fires immediately and blitz-script synthesises `change` from it, the way it already does for a checkbox. A select does not wait for blur the way a text control does. The QA case this unblocks: worktables.dev's schema designer picks its column type and index backend by keyboard or not at all, which is why two composite primary key rules there were implemented but untestable.
Still a character count: the options never reach layout, so nothing shapes them and there is no real measurement to be had. What it counts changes. `text_content().trim()` measured a labelled option's element text, which is the one string the control never shows, and `trim()` only strips the ends, so a label whose own words were split across source lines counted the newline and the indentation before the next word. A picker with a `label` attribute came out sized to hidden markup; one formatted across lines came out wider than the same picker written on one line.
Owner
Author
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.
This is rescued work, not a finished change. Read the caveats before reviewing it as a proposal.
Where it came from
It was found in a temp directory —
/tmp/claude-501/.../scratchpad/blitz-select— on branchfix/qa-engine-defects-2026-09-09, dated by its own name to 2026-09-09. An agent clonedps-blitzthere, did the work, and never pushed it. The commits exist on no remote and in none of the four localps-blitzcheckouts.It surfaced while clearing 809 GB of abandoned cargo build caches out of that same tree. Pushing it is the cheapest way to stop it being one
rm -rffrom gone; it is not a claim that it should merge.What it contains
Four commits, 39 files, 3,427 insertions:
Nine test files come with it:
select_selection,select_keyboard,select_layout,select_accessibility,implicit_form_submission,text_input_change,number_input_sizing,display_block_table,table_row_boxes, plushoisted_child_survives_removalandhover_pointer_move_repaint.Why it is probably not a duplicate
masterhas two select-related commits in the same period —3f60e0aa fix(layout): give <select> a layout boxand6ac25b77 perf(dom): record a radio's set while selecting it. Neither covers selectedness, keyboard interaction,changeevents, the accessibility tree, or implicit form submission. So this is largely ground master does not hold.Why it may have been parked
Stated plainly, because it is the reason to be careful with it:
10cb1a3c(release 0.4.6), and master is 16 commits ahead. It needs a rebase before it means anything./tmpcarries no record of why.What I suggest
Treat this as an archive branch. If the select work is wanted, rebase it onto master and let CI say something about it. If it was parked for a reason, close this and delete the branch — the point was to make the decision visible rather than have the work vanish with a temp directory.