Expose media playback state in snapshots#7
Conversation
Closes #2 Add media-specific state detection in the content script's extractAttrs() for HTMLMediaElement instances, capturing playback state, current time, duration, muted flag, and video resolution. On the Rust side, add a Media node variant to structure.rs with corresponding XML rendering in xml.rs, and handle the new variant in runner.rs and commands.rs match arms.
Add missing closing brace for the function body, which was causing a compilation error due to an unclosed delimiter.
|
@codex review this pr |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f06345856
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| Node::Text { .. } | Node::Heading { .. } => {} | ||
| Node::Text { .. } | Node::Heading { .. } | Node::Media { .. } => {} |
There was a problem hiding this comment.
Node::Media now receives eN IDs and is inserted into element_refs, but this branch excludes media from collect_interactive_targets, which is the only source used by resolve_interactive_target for plugin click/type selectors. As a result, a plugin step cannot resolve a media element even when the page output shows a valid eN ID (or a query like video), so media-related automation fails with “target not found” despite the element being addressable elsewhere.
Useful? React with 👍 / 👎.
|
I checked this branch locally. I do not see an obvious blocking functional issue, and That said, I do think one thing should be tightened up before merge: this PR changes the structuring rules and XML output, and per the repo collaboration rules it would be better to add the minimal necessary tests in In particular, it would help to cover:
The implementation compiles and CI is green, but without tests around output behavior like this, regressions will be harder to catch later.
|
1 participant
Summary
HTMLMediaElementplayback state (paused/playing/ended,currentTime,duration,muted, resolution) in the content script'sextractAttrs()Medianode variant on the Rust side (structure.rs) with dedicated fields<media>elements in XML output (xml.rs)Mediain plugin runner exhaustive matchesTest plan
<audio>or<video>elementsbrowser-cli page <session>and verify media elements appear with state attributespage --freshto confirm state updates--jsonoutput includes media fieldsCloses #2