microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
65.48k stars 3.57k forks source link

[BUG] caret parameter for expect.toHaveScreenshot with forcedColors: 'active' and colorScheme dark does not work. #15211

Open Basti098765 opened 2 years ago

Basti098765 commented 2 years ago

Context:

Code Snippet

 {
      name: 'chromium-highContrast',
      use: {
        browserName: 'chromium',
        colorScheme: 'dark',
        contextOptions: { forcedColors: 'active' },
      },
      grep: /visual/,
      grepInvert: /(gridLayout.spec.ts|modal-overlay-container.spec.ts)/,

    },
  {
      name: 'chromium',
      use: {
        browserName: 'chromium',
        coverage: process.env.COVERAGE_ENABLE ? true : false,
      },
    },

Describe the bug My config looks something like this for the highContrast visual testing. I have various visual tests for inputs wich work fine in normal chromium. When I launch the same visual tests in chromium-highContrast, I see a caret in my Screenshots. If I read the Docs for toHaveScreenshot link It looks like it should be hiden, which it is for my chromium project. Or am i missing something in my setup?

caret? <"hide"|"initial"> When set to "hide", screenshot will hide text caret. When set to "initial", text caret behavior will not be changed. Defaults to "hide".

rwoll commented 2 years ago

If I read the Docs for toHaveScreenshot link It looks like it should be hiden, which it is for my chromium project.

I think it should, too. This seems like a valid test/use case.

Or am i missing something in my setup?

Nope! I think there's a bug in Playwright here. Thanks for the report! I don't have a suggested workaround or fix at the moment.


Minimal Repro

config output
high-contrast renders-1-high-contrast-darwin
default renders-1-default-darwin

Run: npx playwright test with the following files and observe the above output.

// example.spec.ts
import { test, expect } from '@playwright/test';

test('renders', async ({ page }) => {
  await page.setContent('<input type="text">');
  const input = page.locator('input');
  await input.fill('hello');
  await expect(input).toHaveScreenshot();
});
// playwright.config.ts
import type { PlaywrightTestConfig } from '@playwright/test';

const config: PlaywrightTestConfig = {
  projects: [
    { name: 'high-contrast', use: { browserName: 'chromium', colorScheme: 'dark', contextOptions: { forcedColors: 'active' } }},
    { name: 'default', use: { browserName: 'chromium' }},
  ]
};

export default config;
rwoll commented 2 years ago

Looks like Playwright was setting the caret-color to transparent unconditionally; however, in forced colors mode, the options are: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#system_colors. I believe if Playwright sets it to Field under forced color mode, it'll have the desired effect.

rwoll commented 2 years ago

Setting the caret-color inline, appears to have the intended effect of "hiding" the caret: await page.setContent('<input style="caret-color: transparent !important" type="text">');

So this could be a CSS selector specificity issue with the way we are applying the CSS (as PW does not do it inline).

ssams commented 2 years ago

based on the (marked as outdated?) comment from @rwoll, I did a quick test with setting the color to Field and the following looked quite promising to me when inserted in the screenshotter at https://github.com/microsoft/playwright/blob/4112eb815ecccdc926e56051d822c921b78fbc23/packages/playwright-core/src/server/screenshotter.ts#L152-L156

styleTag.textContent = `
    *:not(#playwright-aaaaaaaaaa.playwright-bbbbbbbbbbb.playwright-cccccccccc.playwright-dddddddddd.playwright-eeeeeeeee) {
      caret-color: transparent !important;
    }
    @media (forced-colors: active) {
      *:not(#playwright-aaaaaaaaaa.playwright-bbbbbbbbbbb.playwright-cccccccccc.playwright-dddddddddd.playwright-eeeeeeeee) {
        caret-color: Field !important;
      }
    }
  `;

Any non-system-color will be overwritten in forced-colors mode, so this also applies to transparent iirc, but the additional media query ensures it's manually set to Field if needed. Due to the precedence it also overrides the previous transparent.

The downside of this is that it won't work in situations when forced color adjustments are disabled on an element and a custom background is set. In this case Field won't match the actual background color and it will be visible again. As no method for disabling forced color adjustments for a single CSS property only exists as far as I know (which would allow using transparent without affecting the rest of the element in theory), I don't see a way to make it work in all situations. But I suppose it would already cover most typical use cases, which already seems like a significant improvement.

thewilkybarkid commented 1 year ago

I've encountered the same problem, and @ssams' approach would work for me.

thewilkybarkid commented 1 year ago

I've just had a go at implementing the change, only to find a test based on https://github.com/microsoft/playwright/blob/69a94ed04403568bc6ef31549ca33be28a126e6f/tests/page/page-screenshot.spec.ts#L36-L61 fail in Chromium.

I managed to extract the failure:

image

As the caret is not transparent, it hides part of the letter when visible.

Using caret-shape: underscore would result in fewer collisions, but AFAIK, there's no browser support. Having whitespace before/after the caret would also work, but that involves changing the content.

Is it possible to prevent the blink entirely? Having it always visible is not the same as not showing it, but at least it would be consistent.

aslushnikov commented 1 year ago

As the caret is not transparent, it hides part of the letter when visible.

@thewilkybarkid We force caret color to be transparent, so something else must be going on.

thewilkybarkid commented 1 year ago

@aslushnikov The forced-colors feature resets it; setting it to transparent again doesn't work in Chromium:

@media (forced-colors: active) {
  *:not(#playwright-aaaaaaaaaa.playwright-bbbbbbbbbbb.playwright-cccccccccc.playwright-dddddddddd.playwright-eeeeeeeee) {
    caret-color: transparent !important;
  }
}
aslushnikov commented 1 year ago

The forced-colors feature resets it; setting it to transparent again doesn't work in Chromium:

@thewilkybarkid Oh, that is unfortunate!

Is it possible to prevent the blink entirely? Having it always visible is not the same as not showing it, but at least it would be consistent.

This would require contribution to Chromium and Chrome DevTools Protocol. Some code pointers would be: