Skip to content

fix(core): stop dropping a mounted composition styles, and gate the divergence - #3094

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/composition-assembly-parity
Aug 7, 2026
Merged

fix(core): stop dropping a mounted composition styles, and gate the divergence#3094
miguel-heygen merged 3 commits into
mainfrom
fix/composition-assembly-parity

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Why

A composition mounted as a sub-composition lost its entire stylesheet and scripts whenever they were authored as siblings of the composition root inside its <template>. That shape is legal and common, so three catalog components — oversized-cursor, device-frame-stage, touch-indicator — rendered completely unstyled in the live preview.

oversized-cursor drew its pointer at 1280px against an authored 7cqw (~134px at 1920), because width: 7cqw was never declared at all. Confirmed in the mounted document, where only the host's own <style> was present.

The rendered video was correct the entire time. This was a preview-versus-render divergence, and it survived a fully green test suite.

How

The fix. mountCompositionContent collected assets from the composition root element, so sibling nodes were invisible to it. It now collects from the source node — a superset of the root, and the single point every mount path routes through (external fetch, inline template, nested). It also strips the mounted clone rather than the source: the previous code removed extracted nodes from the node it was handed, which on the inline-template path is a live <template> still in the document, so a remount would have found it emptied.

Why nothing caught it. Every CLI gate — check, lint, validate — reaches the compiler path through bundleToSingleHtml, and the compiler always collected from the whole template. Nothing in the CLI exercises the mount path, which is reachable only through the player and Studio. The repo's own parity test assembled a fixture two ways and deep-equalled a contract across them, but both arms were static-compiler paths — which is exactly why the runtime could drift unnoticed.

The gate. A third arm mounts the same fixture through loadExternalCompositions and extracts the same contract. Three fixtures run through all three arms, one authoring its assets as root siblings — the shape that broke. authoredStyleSignatures was already in the contract and is exactly the signal that was missing, so no contract field was added.

The owner. Both paths answer the same questions — which nodes are a composition's assets, in what order its scripts run, how its CSS is scoped, which head elements hoist, how nested hosts are discovered, which element carries variable defaults. They now have one module to answer them from. It holds decisions only, never I/O: the two paths differ at their boundary in ways that are essential (Node + linkedom + synchronous + strings; browser + fetch + live DOM + script execution), and the runtime ships as a bundle to a CDN, so anything it can reach is weight and risk. Hence zero imports, a structural input type rather than Document, and a test asserting the import surface stays empty.

Routing both paths through that module is deliberately not in this PR — it changes behaviour in four places (below) and belongs where each can be judged and reverted on its own.

Test plan

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (if applicable)

Every claim here was verified in both directions rather than assumed.

The fix's regression test fails on pre-fix code and passes after — run both ways. The parity arm was proven able to fail: with the fix reverted, the sibling fixture fails and names the composition's own scoped selector against an empty list, while the other two fixtures stay green, so the arm is targeted rather than blanket-red. Restored, 7/7 pass.

bun run lint exits 0. Core: 1690 tests passing, plus typecheck:runtime and lint:runtime-preview-guards clean. Producer: 571 tests passing. The shared module's own defect was reproduced by mutation — collecting from the composition root instead of the whole template fails three of its 17 tests, including the sibling case.

Found while doing this, not fixed here

Deriving the shared decisions surfaced four more live divergences, none of them the reported bug, each a behaviour change to decide deliberately:

  • The compiler silently drops inline <head> scripts — it handles the src case and has no else — while the runtime executes them.
  • <link> hoisting is conditional on render and unconditional on mount, so a templated sub-composition's webfont link is dropped in video and kept in preview. This one reproduces under the new parity arm and is explicitly excluded from its contract, with the reason recorded in the file.
  • For a host naming no id, the compiler falls back to the first declared composition and scopes to it; the runtime mounts the content whole, unflattened and unscoped.
  • The compiler keeps two scope ids, CSS and scripts, so a script's self-referencing query resolves when a host names an id the content does not declare; the runtime keeps one.

Separately: the mount path does not recurse at all, so a sub-composition containing its own data-composition-src is silently dropped in live preview. The compiler has a dedicated recursive-discovery suite; the runtime has no nesting, circularity or depth coverage.

Each is recorded with its evidence in the commit messages here, and sequenced so the behaviour-changing ones land separately, after this gate exists to catch a mistake in them.

Not covered

This does not heal the published docs by itself. Previews load @hyperframes/player unpinned, but the player bakes a version-pinned core runtime URL at build time, and core and player publish in lockstep — so the live catalog only recovers after both ship. There is no hotfix path short of a release.

Mounting a composition and rendering one are two implementations of the same
job. They answer the same questions -- which nodes are this composition's
assets, in what order its scripts run, how its CSS is scoped, which head
elements hoist, how nested sub-compositions are discovered, which element
carries variable defaults -- and they answered one of them differently. Assets
authored as siblings of the composition root were collected on render and
dropped on mount, so three catalog components rendered unstyled in live preview
while their video was correct.

This adds the module those answers now live in. No behaviour changes yet; the
next change routes both paths through it.

It holds decisions only, never I/O. The two paths differ at their boundary in
ways that are essential: Node with a linkedom document, synchronous, emitting
strings on one side; a browser that fetches, mutates a live DOM and executes
scripts on the other. The runtime also ships as an esbuild bundle to a CDN, so
anything it can reach is weight and risk. Hence no imports at all, a structural
input type rather than Document, and a test that asserts the import surface
stays empty rather than a comment asking politely.

The shape follows compositionScoping, which is already DOM-free and already
imported by both sides.

Four divergences surfaced while deriving the decisions, all currently live and
none of them the reported bug:

  - the compiler drops inline scripts in a sub-composition head; it handles the
    src case and has no else branch, while the runtime executes them
  - link hoisting is conditional on render and unconditional on mount, so a
    templated sub-composition's webfont link is dropped in video and kept in
    preview
  - for a host that names no id, the compiler falls back to the first declared
    composition in the content and scopes to it; the runtime mounts the content
    whole, unflattened and unscoped
  - the compiler keeps two scope ids, one for CSS and one for scripts, so a
    script's self-referencing query resolves when a host names an id the content
    does not declare; the runtime uses one

The module reports each in the shape the next change will need. Which side wins
is a behaviour decision and is made there, not here.

Verified: 17 tests, and the module's own defect reproduced by mutation --
collecting from the composition root instead of the whole template fails three
of them, including the sibling-asset case. Core suite 1690 passing,
typecheck:runtime and lint:runtime-preview-guards clean, oxlint and oxfmt clean.
A composition mounted as a sub-composition lost its entire stylesheet and
scripts whenever they were authored as siblings of the composition root inside
its template. mountCompositionContent collected assets from the composition root
element, so sibling nodes were invisible to it.

The shape is legal and common, so the result was three catalog components
rendering completely unstyled in live preview. oversized-cursor drew its pointer
at 1280px against an authored 7cqw, roughly 134px at 1920, because width: 7cqw
was never declared at all -- the whole stylesheet was missing. Confirmed in the
mounted document, where only the host's own style element was present.

Collect from the source node, which is a superset of the root and the single
point every mount path routes through: external fetch, inline template, nested.

Strip the mounted clone rather than the source. The previous code removed the
extracted nodes from the node it was given, which on the inline-template path is
a live template still in the document -- a remount would have found it emptied.

Why nothing caught it: every CLI gate reaches the compiler path through
bundleToSingleHtml, and the compiler always collected from the whole template.
Nothing in the CLI exercises the mount path, which is reachable only through the
player and Studio. So the rendered video was correct the entire time and only
the preview was wrong. A test spanning both paths lands separately.

The regression test fails on the pre-fix code, asserting that the mounted
document contains the composition's own container-type declaration and finding
an empty string instead -- the stylesheet that never arrived -- and passes
after. Verified in both directions rather than assumed.
…y contract

The parity test already assembled a fixture two ways and deep-equalled a
contract across them. Both ways were static-compiler paths, which is precisely
why the runtime could drift away from them unnoticed for as long as it did.

This adds the third arm: mount the same fixture through loadExternalCompositions
and extract the same contract from the resulting document. Three fixtures run
through all three arms, and one of them authors its style and script as siblings
of the composition root, which is the shape that broke.

authoredStyleSignatures was already in the contract and is exactly the signal
that was missing, so no contract field was added.

Proven in both directions rather than assumed. With the fix reverted the sibling
fixture fails and names the rule verbatim, reporting the composition's own scoped
selector against an empty list. The other two fixtures stay green, so the arm is
targeted rather than blanket-red. Restored, seven of seven pass, and the full
producer lane is 571 passing.

Two forced deviations. happy-dom rather than jsdom, because jsdom is a
devDependency of core only and does not resolve from producer, while happy-dom is
already a root devDependency used by three other packages; adding jsdom would
have been a lockfile change for no gain. And a deep import of the runtime loader,
because the mount path is deliberately not in core's export map -- it ships
inside the runtime bundle. The engine's frame-extractor test reaches it the same
way, and package-cycles and typecheck both pass.

Two things are asserted rather than compared. The runtime and variable bootstraps
are injected around a mount by the player or producer, never by the loader, so
comparing them would compare harnesses rather than assembly. Every other contract
field is deep-equalled.

One divergence is deliberately outside this gate. A templated sub-composition's
head link is hoisted on mount and dropped on render; that reproduces, and it is a
behaviour decision for a later change rather than something to quietly correct
here. The fixture set covers the non-templated head-link shape, where all three
paths agree, and the exclusion is recorded in the file with its reason.

The fixture set is asserted non-empty and asserted to contain the sibling shape,
so a later refactor cannot empty it and leave a green no-op behind.

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE @ c8c1b0fd252a58487dd415d33ace6f8c10d4a560 — CORRECT / A

What the two fixes are

  1. Stops dropping mounted composition stylesmountCompositionContent now collects <style>/<script> from params.sourceNode (the whole <template> content or doc.body), not from the composition root alone. The canonical authored shape puts assets as SIBLINGS of the root inside <template>, so scanning only the root previously silently dropped a composition's entire stylesheet — #root { container-type: size } never landed, cq* units resolved against the wrong basis, oversized-cursor painted at 1280px instead of ~134px. Render was correct because the compiler path (bundleToSingleHtml) already scanned the whole template.
  2. Gates the divergence with a third parity armhtmlCompiler.parity.test.ts gains a mountContract() arm that runs the same fixture through loadExternalCompositions, parses the resulting live DOM, and extracts the same authoredStyleSignatures contract the compiler arms already compared. Three fixtures × three arms; a meta-test guards that the sibling-shape fixture never gets refactored out (hasRootSiblingAssets scans and asserts siblingShaped.length > 0).

The two changes are load-bearing together: the runtime fix closes the reported defect, and the parity arm is the CI-level gate that makes future compile-vs-mount drift observable. Both pre-existing compiler arms went through bundleToSingleHtml, which is why the runtime could regress under a fully green suite.

Findings

  1. [NIT — nice-to-have] stripExtractedCompositionAssets is called on the two mount paths that clone content (if (innerRoot)flattenedRoot, else if (hasTemplate)mountedContent), but the third else branch at packages/core/src/runtime/compositionLoader.ts:518-519 sets params.host.innerHTML = params.fallbackBodyInnerHtml — a string captured earlier from doc.body.innerHTML at line 716, before any stripping. The extracted <style>/<script> collected from sourceNode = doc.body are also present verbatim in that string, so on this path the composition's styles get injected once (scoped, into document.head) and once again (unscoped, in the host's mounted HTML). Scripts likewise. This is not a regression — the pre-fix code captured fallbackBodyInnerHtml at the same moment and its remainingStyles strip only touched the live doc, so the string always carried the assets — and it is the same "runtime mounts the content whole, unflattened and unscoped" divergence you explicitly catalogued under "Found while doing this, not fixed here." Flagging so a follow-up unit closes it with an intentional decision rather than by accident.

  2. [NOTE] packages/core/src/compiler/compositionAssembly.ts is added with tests but not yet imported by either the compiler path (inlineSubCompositions.ts) or the runtime path (compositionLoader.ts). Your PR body flags this deliberately — "Routing both paths through that module is not in this PR." The behavioural gate against silent drift is therefore the parity third-arm test, not the shared module itself. Worth stating explicitly: until each of the four catalogued behaviour divergences lands routed through this module, the module's planCompositionAssembly contract is decorative (tested but never called in production). Nothing to change here — just a marker to keep the module's follow-ups sequenced so it doesn't rot as a dead surface.

  3. [NIT] hasRootSiblingAssets in packages/producer/src/services/htmlCompiler.parity.test.ts uses !root?.contains(asset) — if a future fixture omits [data-composition-id], root is null and every asset becomes "sibling", inflating the sibling-shape count. Not a false-negative and no current fixture triggers this, but a strict-null guard (root ? !root.contains(asset) : false) matches the semantic "sibling of the composition root" the function claims and would keep the meta-assertion honest under future fixture drift.

  4. [NOTE — approving observation] Test semantics guard the actual shape, not just presence. The runtime unit asserts specific values (toContain("container-type: size"), toContain("__sceneRan"), host.querySelectorAll("style, script").toHaveLength(0), host.querySelector("p")?.textContent === "Scene") and the parity arm compares the full contract (toEqual(assembledContract(result.preview))). Mutation-escape resistance is good. The import-surface test on compositionAssembly.ts (asserting the module has zero imports and no ambient DOM/Node globals) is exactly the right guard for a "must not accrete weight before shipping to the CDN" invariant.

  5. [NOTE — approving observation] The strip-the-clone-never-the-source point is genuinely important — for the inline-template path sourceNode IS a live <template> sitting in the document, so the old strip would have emptied it and any remount would find no styles. The docstring on stripExtractedCompositionAssets at packages/core/src/runtime/compositionLoader.ts:181-193 calls this out explicitly. Reversal-of-a-cache-invalidation trap correctly identified.

Adversarial pass at fix boundary (6 axes)

  • Coordinate/style contract: styles extracted once from sourceNode (superset of root), then stripped from the clone. Idempotent, no double-count.
  • Divergence-gate polarity: "gate" here is a parity-test third arm, not a runtime branch. Polarity is safe by construction — the assertion fails when contracts diverge; there is no default-to-open state.
  • Concurrency/lifecycle: source <template> is never mutated, so a remount re-extracts cleanly. params.injectedStyles accumulates the injected clones for the caller's teardown to sweep.
  • Ambient setup lost in extraction: stripExtractedCompositionAssets is called on both cloning mount paths; the third else path is finding #1 above.
  • Callers of the changed shape: mountCompositionContent has three internal call sites (packages/core/src/runtime/compositionLoader.ts:605, 656, 691); all populate sourceNode correctly (template.content or doc.body). No external consumer.
  • Test asserts guard semantics not presence: covered under Finding 4.

CI

All required checks green at c8c1b0f (Preflight, Build, Lint, Typecheck, Format, Producer: unit tests, Producer: integration tests, Preview parity, Test: runtime contract, Fallow audit, File size check, Perf: parity/drift/fps/load/scrub, Semantic PR title, CodeQL, Analyze). Several regression-shards + Tests-on-windows-latest still pending; historical for Miguel's PRs these are flake-prone but not blocking. Merge only after the pending shards settle green.

— Via

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at c8c1b0fd.

Sharp diagnosis and a clean shape. The bug isn't just "assets missed" — it's the pair "collect from too narrow a subtree" + "strip the source instead of the clone", each mechanically tracked to its symptom (unstyled oversized-cursor; a remounted composition with an emptied <template>). The third arm in the parity contract turns "two static-compiler paths pretending to be different" into a real preview-vs-runtime gate.

The concern worth naming — the shared module isn't wired in yet. packages/core/src/compiler/compositionAssembly.ts has zero non-test consumers (grep -rn "from.*compositionAssembly" returns the test file only). Both the compiler and compositionLoader.ts still inline their own answers to the questions the module claims to own. The PR body acknowledges this deliberately — "Routing both paths through that module is deliberately not in this PR" — but the practical effect is that right now the module is a specification, and the "one owner" claim only becomes an invariant when the four wiring PRs land. Meanwhile a fix landing in one real path won't automatically land in the module or the other, and the parity gate is what catches the mismatch — not a shared source. If it takes several weeks to wire in, the module can silently rot; a note in the module referencing its intended consumers (or a // consumers: compiler/inlineSubCompositions.ts, runtime/compositionLoader.ts header) would help the next reader understand what "owns" means today vs. what it's supposed to mean.

I'd also read the parity gate's "excluded for now, deliberately not worked around" carve-out (htmlCompiler.parity.test.ts:226-231) as a wire that has to be closed before that fixture becomes trustworthy — right now the head-<link> divergence is a listed-but-uncaught failure mode, and the assertion is on the assembled subset. Not a change I'd ask for here, but worth reviewing when U1's next unit lands.

What lands cleanly:

  • The strip-clone-not-source diagnosis in code, not just prose. Pre-PR: script.parentNode?.removeChild(script) and the sibling style.parentNode?.removeChild(style) at compositionLoader.ts:~505-514 mutate the LIVE contentNode, which on the inline-template path is a <template> still in the document. Post-PR: stripExtractedCompositionAssets(flattenedRoot) (:512) and stripExtractedCompositionAssets(mountedContent) (:516) strip clones (document.importNode(..., true)), so a remount finds the template intact. Comment at :184-195 names the failure mode explicitly. That's the sort of "why" comment that pays for itself the next time someone edits this.
  • Widening the collection scope from contentNode to sourceNode (:472 and :494) is exactly the fix: sourceNode is a superset of the composition root by construction (innerRoot is queried from sourceNode.querySelectorAll at :408-409), so nothing is collected twice and everything sibling-of-root is now caught. The comment at :467-471 traces this back to oversized-cursor's #root { container-type: size } never landing — a specific-fault-to-specific-line pointer future reviewers will thank you for.
  • The parity fixture set is protected by a meta-test. htmlCompiler.parity.test.ts:239-245 refuses to shrink below the shape that broke — a refactor that quietly removes the hasRootSiblingAssets-shaped fixture fails the meta before it can hide a regression. Same pattern I want the geometry sweeps in #3092 to grow.
  • The three-arm shape is the right shape. Two compiler arms (bundleToSingleHtml + compileForRender) plus loadExternalCompositions closes the loop the previous parity test couldn't — it was static-vs-static, which is why the runtime drifted without CI noticing. The runtimeBootstrap/variableBootstrap exclusion at :222-224 is honest scoping (both are injected by the harness around the mount, not by the mount itself).
  • The shared module is import-clean and structurally-typed. compositionAssembly.test.ts:251-268 doesn't trust the "no imports" comment — it scans the source and asserts from …, require(, and ambient DOM/Node globals are all empty. Strong guardrail for a module bundled into the CDN runtime. And AssemblyAttributed/AssemblyQueryable<TElement> mean linkedom and the browser DOM both satisfy the input without an as T cast.
  • The four remaining divergences are recorded, not glossed over. The PR body enumerates each with a specific behaviour claim (inline <head> scripts dropped by compiler; <link> hoisting conditional-vs-unconditional; anonymous-host fallback shape divergent; single-vs-two scope-id kept; mount path doesn't recurse). Each is scoped to its own future unit with the rationale in the file for the excluded case. This is the "A/B'd, not assumed" discipline.
  • **The new compositionLoader.test.ts test at :93 covers the specific sibling-of-root shape end-to-end, and asserts the mounted copy has zero style/script tags left over — the strip step's positive invariant, not just a "not crashed" smoke.

One question, purely for my own understanding — not a review point: for a size-tween-anonymous-host case (captions-comp host mounting a captions template, no exact data-composition-id match), the else-branch at :514-517 clones the WHOLE sourceNode (including any authored composition root and its siblings). The strip on the clone removes style/script tags from the mounted tree, and the collected sources are injected separately — so the final mounted DOM ends up with the captions root + any unrelated body content that was next to it. Is that intentional (mount-as-fragment), or is there a case where the compiler's "fall back to the first root and scope to it" would give a different DOM? PR body flags this divergence as one of the four found-not-fixed; happy to defer until that unit lands.

Review by Rames D Jusso

@miguel-heygen
miguel-heygen merged commit 8d9db3d into main Aug 7, 2026
57 checks passed
@miguel-heygen
miguel-heygen deleted the fix/composition-assembly-parity branch August 7, 2026 21:28
miguel-heygen added a commit that referenced this pull request Aug 7, 2026
…es (#3097)

## Why

#3094 fixed one way the mount and render paths disagreed, and added the gate that catches disagreement. It deliberately left the rest.

Four divergences are still live. Each one means a composition assembles differently depending on whether it is being previewed or rendered — the same class of defect that shipped three catalog components unstyled, just with smaller blast radii.

## How

Both paths now derive root discovery, scope identity, asset sources and order, hoisted links, variable carriers and nested-host enumeration from the shared module #3094 introduced. Each keeps its own I/O, which is where they genuinely differ. The compiler's local depth cap and root lookup and the runtime's three pre-filtered head parameters are gone; the runtime hands over the head node and lets the module decide what comes out of it.

Four behaviour changes, each stated by what actually differs rather than by the edit:

**Inline `<head>` scripts.** The compiler looped head scripts with a `src` branch and no `else`, so an inline one was silently discarded on render while the runtime ran it. That is losing code, not holding a convention — the runtime's answer wins. Head and content scripts now share one loop, head first, order preserved. A non-templated sub-composition with an inline head script went from **0 collected scripts to 1**, wrapped, body intact.

**`<link>` hoisting.** Conditional on render, unconditional on mount, so a templated sub-composition's webfont link was dropped in video and kept in preview. Hoisting is the superset and matches what the author declared. A templated composition with a stylesheet link went from **no external links to that link**. The parity fixture that previously recorded this shape as a known exclusion now gates it.

**Anonymous hosts.** With a host naming no id, the compiler fell back to the first declared composition and scoped to it; the mount left the content unflattened and injected its stylesheet into the host `<head>` **unscoped**, so a composition's CSS leaked into whatever mounted it. The compiler's answer wins. The injected rule went from a bare `.label { … }` to `[data-composition-id="scoped-text"] .label { … }`.

**Scope ids.** The compiler splits the CSS scope id from the script composition id; they differ only when a host names an id the content does not declare, and there the scripts follow the declared id so their self-referencing queries resolve. The runtime used one for both. The split wins: a host naming `captions-comp` over content declaring `captions` now emits scripts bound to `captions` while its CSS still scopes to `captions-comp`.

## Test plan

- [x] Unit tests added/updated
- [x] Manual testing performed
- [ ] Documentation updated (if applicable)

Core 1694 passing, producer 574 passing, the parity contract now gates the two divergences it can observe (the other two carry no contract field, so they are gated by unit tests naming the exact before/after). Lint 0, `typecheck:runtime` and the runtime preview guards clean, package cycles unchanged.

Characterization-first: both suites were run and recorded green before any decision moved, so a behavioural drift would surface as a red test rather than a silent difference.

**One assertion changed, deliberately.** A runtime test asserted that an anonymous host's composition is *not* flattened, and documented that as intentional. That premise is now false. What the test actually cared about — the root and its content present under the host — still holds and is still asserted; the "not flattened" claim flipped, and the test now also asserts the scoping that was missing.

## Not covered

The variable-carrier divergence and its `TODO(template-var-carriers)` are untouched by design, as is recursion on the mount path — a sub-composition containing its own `data-composition-src` is still silently dropped in live preview. Both are behaviour changes with their own units, and both are now one-line-ish changes because the shared module already reports what they need.

`runtimeScopeCompositionId` no longer falls back to the authored scope id. This is a functional change beyond the four above, surfaced in review: for an anonymous host with authored variable defaults, the runtime previously stashed them under the declared id, and now does not. It removes a runtime-vs-compiler divergence in the correct direction — the runtime was doing work the compiler never did, and the compiler is authoritative for a shipped composition — but a caller relying on runtime-only variable exposure loses it.

The three copies each of the flattened-root helper and the id assignment are left alone: they look mergeable and are not cheaply, and they touch the instancing contract the pixel harness guards.

## Worth knowing

The parity test's compiler arms import core's **built dist** while the mount arm imports source, so core must be rebuilt before that lane means anything after a compiler change. Skipping it produces a phantom divergence that looks exactly like a real one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants