Skip to content

fix(screencast): register AnnotatePosition enum serializer - #1948

Merged
yury-s merged 2 commits into
microsoft:mainfrom
Develop-KIM:fix-1912
Aug 4, 2026
Merged

fix(screencast): register AnnotatePosition enum serializer#1948
yury-s merged 2 commits into
microsoft:mainfrom
Develop-KIM:fix-1912

Conversation

@Develop-KIM

Copy link
Copy Markdown
Contributor

Summary

  • AnnotatePosition had no Gson serializer registered, so page.screencast().showActions(new Screencast.ShowActionsOptions().setPosition(AnnotatePosition.TOP_RIGHT)) sent the raw constant TOP_RIGHT to the driver, which rejected it: position: expected one of (top-left|top|top-right|...).
  • Registered it with ToLowerCaseAndDashSerializer (as used for ColorScheme, ServiceWorkerPolicy, …) so values serialize as top-right etc.

Fixes #1912

AnnotatePosition had no Gson serializer registered, so
Screencast.showActions() sent the raw enum constant (e.g. TOP_RIGHT) to
the driver, which rejected it with "position: expected one of
(top-left|top|top-right|...)". Register it with ToLowerCaseAndDashSerializer
so values serialize as top-right, matching the other dashed enums.

Fixes: microsoft#1912
@Develop-KIM

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@yury-s yury-s left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change looks good, let's update the test


public class TestSerialization {
@Test
void annotatePositionSerializesToLowerCaseAndDash() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's turn it into and e2e test and check that actual playwright api call doesn't throw page.screencast().showActions(new Screencast.ShowActionsOptions().setPosition(AnnotatePosition.TOP_RIGHT))

@Develop-KIM

Copy link
Copy Markdown
Contributor Author

Done — dropped the serializer unit test and moved the coverage into TestScreencast, going through the real API:

for (AnnotatePosition position : AnnotatePosition.values()) {
  AutoCloseable disposable = page.screencast().showActions(
    new Screencast.ShowActionsOptions().setPosition(position));
  assertNotNull(disposable);
  disposable.close();
}

Ran it against a real browser both ways. Without the serializer registration every position fails with position: expected one of (top-left|top|top-right|bottom-left|bottom|bottom-right), which is the error from #1912; with it the test passes.

I looped over all six values rather than just TOP_RIGHT so a future enum constant that doesn't round-trip gets caught too — happy to narrow it if you'd rather keep the test minimal.

@yury-s
yury-s merged commit 3823986 into microsoft:main Aug 4, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: showActions() with ShowActionsOptions.setPosition() throws protocol error for all AnnotatePosition values

2 participants