Skip to content

[java][bidi] Type field-less synthetic map records as Map<String, V> - #18122

Merged
pujagani merged 38 commits into
SeleniumHQ:trunkfrom
yashp676:java-bidi-synthetic-map
Oct 7, 2026
Merged

pujagani merged 38 commits into
SeleniumHQ:trunkfrom
yashp676:java-bidi-synthetic-map

Conversation

@yashp676

@yashp676 yashp676 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Depends on #17777 — draft until that merges. This branch is rebased on #17777; only the last commit ([java][bidi] Type field-less synthetic map records as Map<String, V>) belongs to this PR.

Related Issues

Java part of #18106 (Ruby part is #18121)

What does this PR do?

script.NodeProperties.attributes ({*text => text} in the CDDL) was generated as an empty nested NodeProperties.Attributes class with @WarnOnUnknownFields, so every attribute was dropped on deserialization. It is now Optional<Map<String, String>>, matching the hand-written NodeProperties API on trunk.

Implementation Notes

  • parseField rewrites a field's ref to a field-less synthetic map record into the inline map typeRef, so resolveJavaType, serializeExpr and immutableCopyExpr all use their existing map branches.
  • I did this at field parsing rather than only in resolveRefType (as suggested in the issue) because serializeExpr and immutableCopyExpr check the field's own typeRef for map. Fixing only the resolved type would give the right Map<String, String> type, but the value would skip the unmodifiable copy and the map serialization path.
  • computeSyntheticChildren skips these records, so no empty nested class is emitted.
  • Scope: primitive-valued maps only, as suggested in the issue. A ref- or union-valued synthetic map keeps its current shape. The only such maps in the schema today are Mozilla vendor types, which this generator does not read.
  • The only generated file that changes is script/NodeProperties.java.

Additional Considerations

Types of changes

  • Bug fix (backwards compatible)

@pujagani
pujagani self-requested a review October 7, 2026 11:30

@pujagani pujagani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. We can do this recursively but it is not needed right now. It might be a good idea to use the JSON decoder that code is now using, it went through few iteration so I understand where this PR is coming from, its not a blocker and I can help do that in the follow up PR.

@pujagani
pujagani merged commit e916a04 into SeleniumHQ:trunk Oct 7, 2026
82 of 84 checks passed
This was referenced Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants