-
Notifications
You must be signed in to change notification settings - Fork 241
fix: Fix masking in single element screenshots. #1825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: Fix masking in single element screenshots. #1825
Conversation
Using masks in single element screenshots (`Locator.screenshot()`) never worked, because the mask option failed to be serialized. When a mask is provided as an option, serialize it manually. This is the same implementation as in `PageImpl.screenshot()`. Fixes microsoft#1790
@microsoft-github-policy-service agree company="Medartis AG" |
" }\n" + | ||
" </style>\n" + | ||
" <div contenteditable=\"true\"></div>\n"); | ||
" stylesheet rules will throw.\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the formatting changes.
@@ -304,13 +315,13 @@ public void scrollIntoViewIfNeeded(ScrollIntoViewIfNeededOptions options) { | |||
|
|||
@Override | |||
public List<String> selectOption(String value, SelectOptionOptions options) { | |||
String[] values = value == null ? null : new String[]{ value }; | |||
String[] values = value == null ? null : new String[]{value}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert code formatting changes here and in other places, they just clutter the PR.
if (mask != null) { | ||
JsonArray maskArray = new JsonArray(); | ||
for (Locator locator : mask) { | ||
maskArray.add(((LocatorImpl) locator).toProtocol()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicating code with PageImpl, let's add a serializer for LocatorImpl.class into Serialization.java, that way it will automatically work for Locator.screenshot, ElementHandle.screenshot and Page.screenshot.
Using masks in single element screenshots (
Locator.screenshot()
) never worked, because the mask option failed to be serialized.When a mask is provided as an option, serialize it manually. This is the same implementation as in
PageImpl.screenshot()
.Fixes #1790