The "limited to only known values" canonicalization (per WHATWG HTML
§2.2.2) was duplicated five times: Form.getMethod + Form.getEnctype +
{Button,Input}.{getFormMethod,getFormEnctype}. Each callsite differed
only in the missing-value default ("" for submitter overrides, "get" /
"application/x-www-form-urlencoded" for the form-side).
Extract into two pub helpers on Form.zig taking the attribute slice +
the missing-value default. The five callers collapse to one-liners.
Behavior-preserving: existing form.html / button.html / input-attrs.html
fixtures all pass unchanged; full suite 637/637 green.
Net: -36 LOC.
Six form-submission IDL accessors were missing from the JsApi blocks of
HTMLFormElement, HTMLButtonElement, and HTMLInputElement, so reads
produced undefined instead of the spec-mandated string/boolean. The
content-attribute path (clicking a submit button honoring formaction /
formmethod / formenctype) was wired up in #2279; this commit adds the
matching IDL-property accessors per WHATWG HTML §4.10.18.6 and §4.10.21.5.
- Form.enctype: limited to known values, missing+invalid both default to
application/x-www-form-urlencoded (mirrors getMethod's shape).
- Button/Input formAction: returns frame.url when missing/empty, else the
resolved URL (mirrors Form.getAction).
- Button/Input formEnctype, formMethod: limited to known values with no
missing-value default ("" when missing, canonical invalid-value default
application/x-www-form-urlencoded / get when invalid).
- Button/Input formTarget: plain reflection, defaults to "".
- Button/Input formNoValidate: boolean reflection of formnovalidate.
Closes#2449
At some point recently, we started to process scripts that fail to load (e.g.
404). This stops such scripts from [trying] to be evaluated, and executes the
onerror handler in all script loading paths.
First, KeyValueList.fromJsObject now only iterates own properties. Second
URLSearchParams can now be constructed with another URLSearchParams. This is
a stopgap. The correct solution is for it to accept any iterator, but as a
quick fix for known cases (airbnb.com), this will help.
Adds two ways to opt out of dedicated Web Worker loading entirely. The
Worker constructor still returns a Worker object so calling pages don't
throw, but no script fetch is initiated and the worker scope's eval
never runs (postMessage from the page is queued indefinitely with no
handler to drain it).
* CDP method LP.configureLoading { worker: bool } -- per-session
toggleable at runtime, alongside the existing { subFrame: bool }.
Both fields are now optional so callers can flip one without
resetting the other to its default. Backwards-compatible.
* CLI flag --disable-workers -- process-wide default applying to every
session and to the fetch subcommand. Operators can flip it on without
any driver changes. Mirrors --disable-subframes (#2401) exactly.
## Motivation
Reliably-reproducible SIGABRT in Worker.loadInitialScript whenever a
page constructs a Web Worker AND lightpanda is launched with
--http_proxy. Crash signature:
$msg="V8 fatal callback" location=v8::Context::Exit()
message="Cannot exit non-entered context"
Stack:
_browser.webapi.Worker.loadInitialScript
_browser.webapi.Worker.httpDoneCallback
_network.layer.InterceptionLayer.InterceptContext.doneCallback
_browser.HttpClient.processMessages
_browser.HttpClient.perform
_browser.HttpClient.tick
The Zig-side Enter/Exit pair around the worker's eval doesn't match
v8's entered_contexts stack invariant under that timing -- something
upstream of the loadInitialScript Exit leaves an extra Enter on the
stack, so v8's Utils::ApiCheck (`isolate->context() == *env`) fires
and the process aborts.
Reproducible against any Shopify storefront PDP (e.g.
https://weareallbirds.myshopify.com/products/mens-wool-runners) when
served through any HTTP proxy -- the proxy just adds enough latency
to surface the race; the same code path runs without --http_proxy
but the timing window is too tight to reliably hit. The Allbirds
trigger script is the Shopify web-pixel-extension worker, but ANY
Worker the page constructs hits the same code path.
The proper fix needs the v8 entered-contexts invariant to be
restored end-to-end through the worker eval. That's a deeper dig
into how Worker.loadInitialScript / WorkerGlobalScope.importScript /
ls.local.runMacrotasks compose with v8's microtask queues across
multiple contexts; I tried three intermediate fixes (deferring
loadInitialScript via the frame scheduler when other scripts are
mid-eval, replacing the post-eval cross-context runMacrotasks with
worker-only PerformCheckpoint, and removing runMacrotasks entirely)
and none stopped the crash. The bug is fired from inside the
synchronous tick path before the post-eval microtask handling
runs, which means the leak happens during Script::Run itself and
needs more targeted investigation.
This PR is the workaround so users hitting the SIGABRT on
storefront / analytics-heavy pages have a clean opt-in escape today.
For our use case (product catalog extraction) Workers carry no
extraction signal -- web-pixel sandboxes, analytics SDKs, marketing
tag pixels, etc. -- so disabling them removes a fragile code path
without any downside.
## Implementation
`Session.worker_loading_enabled: bool = true` -- default matches
existing behavior.
`Worker.init` short-circuits AFTER constructing the Worker /
WorkerGlobalScope / arena bookkeeping (so the JS `new Worker(url)`
expression doesn't throw):
if (!session.worker_loading_enabled) {
log.debug(.browser, "worker disabled", .{ .url = resolved_url });
return self;
}
Two ways to flip the flag, mirroring the --disable-subframes pattern:
1. LP.configureLoading { worker: bool } -- both subFrame and worker
are now optional fields in the params struct, so existing callers
passing only { subFrame } continue to work unchanged.
2. --disable-workers CLI flag -- added to CommonOptions (so it
applies to serve, fetch, mcp). New Config.disableWorkers() getter;
Session.init reads it as the initial value.
Total diff: +88 / -3 across 4 files (src/Config.zig,
src/browser/Session.zig, src/browser/webapi/Worker.zig,
src/cdp/domains/lp.zig).
## Verification
Reproducer pattern (puppeteer-core 24.42.0 + tiny CONNECT-tunnel
proxy on 127.0.0.1:9999, scripts in cdp-repros/):
serve --host 127.0.0.1 --port 9222 --http_proxy http://127.0.0.1:9999
serve --host 127.0.0.1 --port 9222 --http_proxy http://127.0.0.1:9999 --disable-workers
Driving https://weareallbirds.myshopify.com/products/mens-wool-runners:
baseline (no --disable-workers): 5/5 SIGABRT in
Worker.loadInitialScript with the v8 fatal callback above.
with --disable-workers: 10/10 successful, returns full
HTML (~1MB), no crash.
Test suite:
make test -> 637 of 637 tests passed (was 636/636 + new
cdp.lp: configureLoading toggles subFrame and worker
independently regression test).
zig fmt --check ./*.zig ./**/*.zig -> clean.
## Notes
* The CDP method is the same domain (LP.configureLoading) and same
shape as --disable-subframes' driver-side opt-in, so existing
Playwright / puppeteer integrations that already toggle
subframes don't need a separate code path -- one CDP call can
flip both.
* worker_loading_enabled = false does NOT remove Worker from the
global namespace (so feature-detection like
`if (typeof Worker !== 'undefined')` still reports true). It just
makes constructed workers no-op. Pages that postMessage to a worker
and wait for a response will hang on that promise forever (or
until the page is torn down). For our extraction use case that's
fine -- we control the worklist timeout anyway -- but it's worth
noting if upstream wants to surface the disabled state more
strongly (e.g. throw from postMessage, or remove the global
entirely behind an even-stricter flag).
* Once the underlying v8 entered-contexts invariant is restored in
Worker.loadInitialScript, this flag becomes a perf / sandboxing
tool rather than a correctness workaround. Worth keeping anyway:
blocking analytics / pixel workers is a reasonable thing to want.
## Related
* #2400 -- the iframe analog to this issue (subframe nav invalidates
executionContextId); same workaround pattern.
* #2401 -- introduced --disable-subframes / LP.configureLoading
{ subFrame } that this PR mirrors exactly for workers.
The HTMLDialogElement constructor was exposed with `open` / `returnValue`
IDL accessors, but the three instance methods that drive the open/close
state were missing. Per HTML §4.11.4 (The dialog element), `show()` sets
the `open` attribute if absent; `showModal()` throws `InvalidStateError`
when the dialog is already open and otherwise sets `open`; `close()`
removes `open`, optionally updates `returnValue`, and fires a non-
bubbling `close` event. The non-rendering steps (focus trap, backdrop,
top-layer placement) are intentional no-ops here — `[open]` reflecting
through to selectors and the `close` event firing are what downstream
CDP clients rely on.
Closes#2434
The build script wrote the version line to stdout, polluting any pipeline
that captures program output via 'zig build run' or similar. Banners
belong on stderr.
The frameNavigated CDP handler sent Page.frameNavigated twice per
navigation and always used the root frame's V8 context for
inspector.contextCreated, even during iframe navigations. This caused
the root frame's inspector context id to be silently re-registered
(and the previous id invalidated) whenever an iframe navigated.
The duplicate Page.frameNavigated masked this bug — Puppeteer happened
to pick up the re-registered context id. Removing the duplicate exposed
the underlying issue: callFunctionOn with the original id failed with
"Cannot find context with specified id".
1 - Track owner of a request (for simpler / more accurate abort (TBD))
2 - Create Transfer upfront, make everything work on Transfer (not Request)
This helps remove ambiguity about cleanup and simplifies layers. For example
Robots request is just another normal request, not a special case. This gives
everything a stable address (the *Transfer which can be looked up by id)
These are currently reachable in Worker via the OffscreenCanvas, so when used
they crash. WPT /html/canvas/offscreen/2d.conformance.requirements.basics.worker.html
