Skip to content

Commit 85011e9

Browse files
janne-hyoetylae-medjanne-hyoetylae-med
committed
fix: Fix masking in single element screenshots.
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
1 parent 0cf8c4e commit 85011e9

File tree

2 files changed

+60
-38
lines changed

2 files changed

+60
-38
lines changed

‎playwright/src/main/java/com/microsoft/playwright/impl/ElementHandleImpl.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.gson.JsonObject;
2222
import com.microsoft.playwright.ElementHandle;
2323
import com.microsoft.playwright.Frame;
24+
import com.microsoft.playwright.Locator;
2425
import com.microsoft.playwright.PlaywrightException;
2526
import com.microsoft.playwright.options.BoundingBox;
2627
import com.microsoft.playwright.options.ElementState;
@@ -44,7 +45,7 @@ public class ElementHandleImpl extends JSHandleImpl implements ElementHandle {
4445

4546
ElementHandleImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) {
4647
super(parent, type, guid, initializer);
47-
this.frame = (FrameImpl)parent;
48+
this.frame = (FrameImpl) parent;
4849
}
4950

5051
@Override
@@ -282,8 +283,18 @@ public byte[] screenshot(ScreenshotOptions options) {
282283
}
283284
}
284285
}
286+
List<Locator> mask = options.mask;
287+
options.mask = null;
285288
JsonObject params = gson().toJsonTree(options).getAsJsonObject();
289+
options.mask = mask;
286290
params.remove("path");
291+
if (mask != null) {
292+
JsonArray maskArray = new JsonArray();
293+
for (Locator locator : mask) {
294+
maskArray.add(((LocatorImpl) locator).toProtocol());
295+
}
296+
params.add("mask", maskArray);
297+
}
287298
JsonObject json = sendMessage("screenshot", params, frame.timeout(options.timeout)).getAsJsonObject();
288299

289300
byte[] buffer = Base64.getDecoder().decode(json.get("binary").getAsString());
@@ -304,13 +315,13 @@ public void scrollIntoViewIfNeeded(ScrollIntoViewIfNeededOptions options) {
304315

305316
@Override
306317
public List<String> selectOption(String value, SelectOptionOptions options) {
307-
String[] values = value == null ? null : new String[]{ value };
318+
String[] values = value == null ? null : new String[]{value};
308319
return selectOption(values, options);
309320
}
310321

311322
@Override
312323
public List<String> selectOption(ElementHandle value, SelectOptionOptions options) {
313-
ElementHandle[] values = value == null ? null : new ElementHandle[]{ value };
324+
ElementHandle[] values = value == null ? null : new ElementHandle[]{value};
314325
return selectOption(values, options);
315326
}
316327

@@ -328,7 +339,7 @@ public List<String> selectOption(String[] values, SelectOptionOptions options) {
328339

329340
@Override
330341
public List<String> selectOption(SelectOption value, SelectOptionOptions options) {
331-
SelectOption[] values = value == null ? null : new SelectOption[]{ value };
342+
SelectOption[] values = value == null ? null : new SelectOption[]{value};
332343
return selectOption(values, options);
333344
}
334345

‎playwright/src/test/java/com/microsoft/playwright/TestPageScreenshot.java

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ static private void rafraf(Page page) {
6969
// Do a double raf since single raf does not
7070
// actually guarantee a new animation frame.
7171
page.evaluate("() => new Promise(x => {\n" +
72-
" requestAnimationFrame(() => requestAnimationFrame(x));\n" +
73-
" })");
72+
" requestAnimationFrame(() => requestAnimationFrame(x));\n" +
73+
" })");
7474
}
7575

7676
@Test
@@ -136,7 +136,7 @@ void shouldNotCaptureInfiniteWebAnimations() {
136136
}
137137

138138
@Test
139-
void maskShouldWork() {
139+
void maskShouldWorkForPage() {
140140
page.setViewportSize(500, 500);
141141
page.navigate(server.PREFIX + "/grid.html");
142142
byte[] screenshot = page.screenshot(new Page.ScreenshotOptions()
@@ -146,6 +146,17 @@ void maskShouldWork() {
146146
assertThrows(AssertionFailedError.class, () -> assertArrayEquals(screenshot, originalScreenshot));
147147
}
148148

149+
@Test
150+
void maskShouldWorkForLocator() {
151+
page.navigate(server.PREFIX + "/grid.html");
152+
Locator locatorToScreenshot = page.locator("div").first();
153+
byte[] screenshot = locatorToScreenshot.screenshot(new Locator.ScreenshotOptions()
154+
.setMask(asList(page.locator("img"))));
155+
// TODO: toMatchSnapshot is not present in java, so we only checks that masked screenshot is different.
156+
byte[] originalScreenshot = locatorToScreenshot.screenshot();
157+
assertThrows(AssertionFailedError.class, () -> assertArrayEquals(screenshot, originalScreenshot));
158+
}
159+
149160
@Test
150161
void shouldWorkWithDeviceScaleFactorAndClip() {
151162
try (BrowserContext context = browser.newContext(new Browser.NewContextOptions()
@@ -186,16 +197,16 @@ void shouldWorkWithDeviceScaleFactorAndScaleDevice() {
186197
@Test
187198
void shouldNotCaptureBlinkingCaretByDefault() {
188199
page.setContent("<!-- Refer to stylesheet from other origin. Accessing this\n" +
189-
" stylesheet rules will throw.\n" +
190-
" -->\n" +
191-
" <link rel=stylesheet href=\"" + server.CROSS_PROCESS_PREFIX + "/injectedstyle.css\">\n" +
192-
" <!-- make life harder: define caret color in stylesheet -->\n" +
193-
" <style>\n" +
194-
" div {\n" +
195-
" caret-color: #000 !important;\n" +
196-
" }\n" +
197-
" </style>\n" +
198-
" <div contenteditable=\"true\"></div>\n");
200+
" stylesheet rules will throw.\n" +
201+
" -->\n" +
202+
" <link rel=stylesheet href=\"" + server.CROSS_PROCESS_PREFIX + "/injectedstyle.css\">\n" +
203+
" <!-- make life harder: define caret color in stylesheet -->\n" +
204+
" <style>\n" +
205+
" div {\n" +
206+
" caret-color: #000 !important;\n" +
207+
" }\n" +
208+
" </style>\n" +
209+
" <div contenteditable=\"true\"></div>\n");
199210
Locator div = page.locator("div");
200211
div.type("foo bar");
201212
byte[] screenshot = div.screenshot();
@@ -210,19 +221,19 @@ void shouldNotCaptureBlinkingCaretByDefault() {
210221
}
211222

212223
@Test
213-
@DisabledIf(value="com.microsoft.playwright.TestBase#isFirefox", disabledReason="fixme")
224+
@DisabledIf(value = "com.microsoft.playwright.TestBase#isFirefox", disabledReason = "fixme")
214225
void shouldCaptureBlinkingCaretIfExplicitlyAskedFor() {
215226
page.setContent(" <!-- Refer to stylesheet from other origin. Accessing this\n" +
216-
" stylesheet rules will throw.\n" +
217-
" -->\n" +
218-
" <link rel=stylesheet href=\"" + server.CROSS_PROCESS_PREFIX + "/injectedstyle.css'}\">\n" +
219-
" <!-- make life harder: define caret color in stylesheet -->\n" +
220-
" <style>\n" +
221-
" div {\n" +
222-
" caret-color: #000 !important;\n" +
223-
" }\n" +
224-
" </style>\n" +
225-
" <div contenteditable=\"true\"></div>\n");
227+
" stylesheet rules will throw.\n" +
228+
" -->\n" +
229+
" <link rel=stylesheet href=\"" + server.CROSS_PROCESS_PREFIX + "/injectedstyle.css'}\">\n" +
230+
" <!-- make life harder: define caret color in stylesheet -->\n" +
231+
" <style>\n" +
232+
" div {\n" +
233+
" caret-color: #000 !important;\n" +
234+
" }\n" +
235+
" </style>\n" +
236+
" <div contenteditable=\"true\"></div>\n");
226237
Locator div = page.locator("div");
227238
div.type("foo bar");
228239
byte[] screenshot = div.screenshot();
@@ -265,32 +276,32 @@ static boolean isScreenshotTestDisabled() {
265276
}
266277

267278
@Test
268-
@DisabledIf(value="com.microsoft.playwright.TestPageScreenshot#isScreenshotTestDisabled", disabledReason="array lengths differ")
279+
@DisabledIf(value = "com.microsoft.playwright.TestPageScreenshot#isScreenshotTestDisabled", disabledReason = "array lengths differ")
269280
void shouldHideElementsBasedOnAttr() throws IOException {
270281
page.setViewportSize(500, 500);
271282
page.navigate(server.PREFIX + "/grid.html");
272283
page.locator("div").nth(5).evaluate("element => {\n" +
273-
" element.setAttribute('data-test-screenshot', 'hide');\n" +
274-
"}");
284+
" element.setAttribute('data-test-screenshot', 'hide');\n" +
285+
"}");
275286
byte[] actual = page.screenshot(new Page.ScreenshotOptions().setStyle("[data-test-screenshot=\"hide\"] {\n" +
276-
" visibility: hidden;\n" +
277-
" }"));
287+
" visibility: hidden;\n" +
288+
" }"));
278289
assertArrayEquals(expectedScreenshot("hide-should-work"), actual, "Screenshots should match");
279290
Object visibility = page.locator("div").nth(5).evaluate("element => element.style.visibility");
280291
assertEquals("", visibility);
281292
}
282293

283294
@Test
284-
@DisabledIf(value="com.microsoft.playwright.TestPageScreenshot#isScreenshotTestDisabled", disabledReason="array lengths differ")
295+
@DisabledIf(value = "com.microsoft.playwright.TestPageScreenshot#isScreenshotTestDisabled", disabledReason = "array lengths differ")
285296
void shouldRemoveElementsBasedOnAttr() throws IOException {
286297
page.setViewportSize(500, 500);
287298
page.navigate(server.PREFIX + "/grid.html");
288299
page.locator("div").nth(5).evaluate("element => {\n" +
289-
" element.setAttribute('data-test-screenshot', 'remove');\n" +
290-
" }");
300+
" element.setAttribute('data-test-screenshot', 'remove');\n" +
301+
" }");
291302
byte[] actual = page.screenshot(new Page.ScreenshotOptions().setStyle("[data-test-screenshot=\"remove\"] {\n" +
292-
" display: none;\n" +
293-
" }"));
303+
" display: none;\n" +
304+
" }"));
294305
assertArrayEquals(expectedScreenshot("remove-should-work"), actual, "Screenshots should match");
295306
Object display = page.locator("div").nth(5).evaluate("element => element.style.display");
296307
assertEquals("", display);

0 commit comments

Comments
 (0)