LibWeb: Implement some missing IDL interface members - #10967
Conversation
The active view transition is already tracked on the document. It is now reachable from script.
Transition events are already dispatched. They could only be observed through addEventListener until now.
The attribute behaves identically on both style elements. Its implementation now lives on the shared style element base. SVG 2 declares the attribute without defining its behavior, so it follows the HTML `style` element.
The method returns a new, detached `SVGNumber` whose value is zero.
Transition events now carry the `CSSTransition` that produced them.
The attribute reports whether the user agent displayed a cached rendered state of the entry being traversed to.
The attribute carries the data being inserted for input types that insert rich content. It is initialized from the event init dictionary.
📝 WalkthroughWalkthroughThe change adds CSS transition animation references, transition handlers, view-transition and history event properties, stylesheet controls, SVG factories, and ChangesWeb platform API additions
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Tests/LibWeb/Text/input/wpt-import/input-events/idlharness.window.js (1)
10-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a behavior test for constructor initialization.
The current harness creates only
new InputEvent("foo"). It verifies thatdataTransferexists, but it does not verify that a suppliedInputEventInit.dataTransferis preserved or that the default isnull.Add a LibWeb-owned regression test for both cases. Based on learnings, keep the imported WPT fixture focused on upstream IDL coverage and place the behavior test outside
Tests/LibWeb/Text/input/wpt-import/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/Text/input/wpt-import/input-events/idlharness.window.js` around lines 10 - 12, Add a separate LibWeb-owned behavior test outside the imported WPT fixture directory that constructs InputEvent with an InputEventInit dataTransfer value and asserts it is preserved, then constructs one without dataTransfer and asserts the property defaults to null. Keep the existing idl_array setup in input-events/idlharness.window.js limited to upstream IDL coverage.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/UIEvents/InputEvent.idl`:
- Around line 19-21: The InputEventInit definition and construction path omit
targetRanges. Add the sequence<StaticRange> targetRanges member with its empty
default to InputEventInit in InputEvent.idl, then update
InputEvent::construct_impl() to read and pass it into the InputEvent constructor
so getTargetRanges() preserves values supplied to new InputEvent().
In `@Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/idlharness.txt`:
- Around line 38-45: Update the imported CSS Transitions IDL definition for
TransitionEvent to include the animation attribute, then regenerate the
corresponding idlharness expected output so it contains the animation attribute
and inherited-property checks; do not edit the generated expected file before
updating the IDL source.
---
Nitpick comments:
In `@Tests/LibWeb/Text/input/wpt-import/input-events/idlharness.window.js`:
- Around line 10-12: Add a separate LibWeb-owned behavior test outside the
imported WPT fixture directory that constructs InputEvent with an InputEventInit
dataTransfer value and asserts it is preserved, then constructs one without
dataTransfer and asserts the property defaults to null. Keep the existing
idl_array setup in input-events/idlharness.window.js limited to upstream IDL
coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fd32c28-e54d-4668-a9d8-bcb28b340c47
📒 Files selected for processing (42)
Libraries/LibWeb/CSS/TransitionEvent.cppLibraries/LibWeb/CSS/TransitionEvent.hLibraries/LibWeb/CSS/TransitionEvent.idlLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.idlLibraries/LibWeb/DOM/StyleElementBase.cppLibraries/LibWeb/DOM/StyleElementBase.hLibraries/LibWeb/HTML/AttributeNames.hLibraries/LibWeb/HTML/GlobalEventHandlers.hLibraries/LibWeb/HTML/HTMLStyleElement.cppLibraries/LibWeb/HTML/HTMLStyleElement.hLibraries/LibWeb/HTML/PopStateEvent.cppLibraries/LibWeb/HTML/PopStateEvent.hLibraries/LibWeb/HTML/PopStateEvent.idlLibraries/LibWeb/SVG/SVGSVGElement.cppLibraries/LibWeb/SVG/SVGSVGElement.hLibraries/LibWeb/SVG/SVGSVGElement.idlLibraries/LibWeb/SVG/SVGStyleElement.idlLibraries/LibWeb/UIEvents/InputEvent.cppLibraries/LibWeb/UIEvents/InputEvent.hLibraries/LibWeb/UIEvents/InputEvent.idlTests/LibWeb/TestConfig.iniTests/LibWeb/Text/expected/SVG/svg-create-svg-number.txtTests/LibWeb/Text/expected/SVG/svg-style-element-disabled.txtTests/LibWeb/Text/expected/wpt-import/css/css-transitions/events-008.txtTests/LibWeb/Text/expected/wpt-import/css/css-transitions/idlharness.txtTests/LibWeb/Text/expected/wpt-import/css/css-transitions/transitionevent-interface.txtTests/LibWeb/Text/expected/wpt-import/css/css-view-transitions/document-active-view-transition.txtTests/LibWeb/Text/expected/wpt-import/html/browsers/browsing-the-web/history-traversal/PopStateEvent.txtTests/LibWeb/Text/expected/wpt-import/input-events/idlharness.window.txtTests/LibWeb/Text/expected/wpt-import/svg/idlharness.window.txtTests/LibWeb/Text/input/SVG/svg-create-svg-number.htmlTests/LibWeb/Text/input/SVG/svg-style-element-disabled.htmlTests/LibWeb/Text/input/wpt-import/css/css-transitions/events-008.htmlTests/LibWeb/Text/input/wpt-import/css/css-transitions/idlharness.htmlTests/LibWeb/Text/input/wpt-import/css/css-transitions/transitionevent-interface.htmlTests/LibWeb/Text/input/wpt-import/css/css-view-transitions/document-active-view-transition.htmlTests/LibWeb/Text/input/wpt-import/html/browsers/browsing-the-web/history-traversal/PopStateEvent.htmlTests/LibWeb/Text/input/wpt-import/input-events/idlharness.window.htmlTests/LibWeb/Text/input/wpt-import/input-events/idlharness.window.jsTests/LibWeb/Text/input/wpt-import/interfaces/css-transitions.idlTests/LibWeb/Text/input/wpt-import/interfaces/input-events.idl
💤 Files with no reviewable changes (2)
- Libraries/LibWeb/HTML/HTMLStyleElement.cpp
- Libraries/LibWeb/HTML/HTMLStyleElement.h
Target ranges given to the `InputEvent` constructor were discarded. Scripts saw an empty list from `getTargetRanges()`.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/LibWeb/Text/input/UIEvents/InputEvent-construction-target-ranges.html (1)
5-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
InputEventInit.dataTransfer.This test covers only
targetRanges. Add a case with a non-nullDataTransferand verify thatevent.dataTransferreturns the expected value. If another test already covers this path, confirm that coverage instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/Text/input/UIEvents/InputEvent-construction-target-ranges.html` around lines 5 - 24, Extend the existing InputEvent construction test around the targetRanges case to create a non-null DataTransfer, pass it through InputEventInit.dataTransfer, and verify event.dataTransfer returns that same object. If existing coverage already validates this behavior, confirm it instead of adding duplicate coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/LibWeb/Text/input/UIEvents/InputEvent-construction-target-ranges.html`:
- Around line 5-24: Extend the existing InputEvent construction test around the
targetRanges case to create a non-null DataTransfer, pass it through
InputEventInit.dataTransfer, and verify event.dataTransfer returns that same
object. If existing coverage already validates this behavior, confirm it instead
of adding duplicate coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 098982d6-b463-43b4-bcf8-4a2969e5c326
📒 Files selected for processing (4)
Libraries/LibWeb/UIEvents/InputEvent.cppLibraries/LibWeb/UIEvents/InputEvent.idlTests/LibWeb/Text/expected/UIEvents/InputEvent-construction-target-ranges.txtTests/LibWeb/Text/input/UIEvents/InputEvent-construction-target-ranges.html
🚧 Files skipped from review as they are similar to previous changes (1)
- Libraries/LibWeb/UIEvents/InputEvent.idl
This PR adds a smattering of APIs, which are supported by every other engine and were easily implementable.
Found by scouring: https://cxbyte.me/are-we-web-yet.html.
See individual commits for details.
NB: A few of the
css-view-transitionsWPTs complained when I tried to import them because they were referencing missing scripts. I've put up a fix for that here (web-platform-tests/wpt#61680). I've stripped the missing scripts from the imports here too.