[Workers] Document two event spec compliance compatibility flags - #32777
[Workers] Document two event spec compliance compatibility flags#32777rexxars wants to merge 1 commit into
Conversation
Review
👉 Fix in your agent 👈Fix the following review findings in PR #32777 (https://github.com/cloudflare/cloudflare-docs/pull/32777).
Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order
After triaging, follow this order:
1. Post a comment on this PR for any findings you are skipping, with the finding ID and your reasoning.
2. Then commit the fixes for the legitimate findings.
The comment must come before the commit — the bot reads PR comments when a new
push triggers a review, so skip comments posted after the push will be missed.
---
## Conventions
### Warnings (1)
#### CV-ef3b8a5ba845 · Scope accuracy
- **File:** PR-level finding
- **Issue:** The PR description lists three compatibility flags and says "Each page covers...", implying three new pages, but `args.changedFiles` only shows two new files (`spec-compliant-event-handler-attributes.md` and `spec-compliant-message-event-origin.md`). The third flag, `spec_compliant_event_is_trusted`, has no corresponding changed file.
- **Fix:** Add the missing `spec-compliant-event-is-trusted.md` page, or update the description to clearly state which flags are actually included in this PR.
Code ReviewThis code review is in beta and may not always be helpful — use your judgment. No code review issues found. ConventionsWarnings (1)
Style Guide ReviewNo style-guide issues found. CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
CR-1127b6e4aab4 (Incorrect addEventListener return-value behavior)Disagree on the factual claim, but rewording anyway, because the sentence is ambiguous enough that a reader could reach the same conclusion. The claim is that return-true-cancels "applies only to the old
The DOM standard part of the finding is correct: a listener's return value is ignored, and never cancels. That is exactly why #7019 leaves So the docs sentence was describing the runtime's rule, not the standard's. Rewording it to say that outright, since "which is the rule the Workers runtime applies" reads as a claim about expected behavior:
Pushing that now. |
3fb866f to
8cff6ce
Compare
8cff6ce to
adaf92b
Compare
Summary
Documents two new compatibility flags added to the Workers runtime in cloudflare/workerd#7019. Both fix places where our event classes deviate from the DOM and HTML standards.
spec_compliant_event_handler_attributes:on<type>handlers, such asWebSocket.onmessageandAbortSignal.onabort, fire in the order they were assigned rather than ahead of every listener, and fire once rather than twice for a class that implements them by registering a listener. Fixes EventTarget is broken and not aligned with spec workerd#6022.spec_compliant_message_event_origin:MessageEvent.originreports an empty string rather thannullwhen there is no origin, and aWebSocketopened from a URL reports the origin of that URL.Each page covers what changes, a before and after example, and the disable flag.
Neither has an
enable_dateyet, so both pages omit that field and use the date they were written assort_date. The flags are opt-in until the runtime PR settles on dates, and I will update this PR with them at the same time as the flag definitions. This goes up alongside the runtime change becausedocs/api-updates.mdin workerd asks for the documentation to land before the enable date.The runtime PR carries a third fix, making
isTrustedfalse on events constructed in JavaScript, which has no compatibility flag after review feedback that it did not need one. There is no page for it here, since these pages document flags.No changelog entry for the two flags, since nothing changes for an existing Worker until they have enable dates. The
isTrustedfix does take effect when the runtime PR merges, so tell me if you would like a changelog entry for that one and I will add it.Documentation checklist