Skip to content
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

[BUG] <button> in shadow DOM not working with click() #5858

Closed
thernstig opened this issue Mar 17, 2021 · 18 comments · Fixed by #6127
Closed

[BUG] <button> in shadow DOM not working with click() #5858

thernstig opened this issue Mar 17, 2021 · 18 comments · Fixed by #6127
Assignees

Comments

@thernstig
Copy link
Contributor

Context:

  System:
    OS: Linux 5.4 Ubuntu 20.04.1 LTS (Focal Fossa)
    Memory: 10.23 GB / 31.34 GB
    Container: Yes
  Binaries:
    Node: 14.15.1 - ~/.local/share/nvm/v14.15.1/bin/node
    npm: 6.14.8 - ~/.local/share/nvm/v14.15.1/bin/npm
  Languages:
    Bash: 5.0.17 - /usr/bin/bash
  npmPackages:
    playwright: 1.9.2 => 1.9.2
    playwright-core: 1.9.2 => 1.9.2
  • Browser: Chromium

Code Snippet

Proprietary project, but I got a nice picture.

picture

// this fails, it hangs here and times out
await page.click('#apply-button button');

Describe the bug

I checked with the (awesome!) Inspector and saw this.

retrying click action, attempt #1
waiting for element to be visible, enabled and stable
element is visible, enabled and stable
scrolling into view if needed
done scrolling
checking that element receives pointer events at (734.03,526)
<xxxx-base-v0-button primary="" slot="bottom" id="apply-button"></xxx-base-v0-button> intercepts pointer events
retrying click action, attempt #2

So it seems to be visible, enabled and stable, but gets stuck at "intercepts pointer events". I have never had this problem before, and I've written many tests before. What could be wrong?

@yury-s
Copy link
Member

yury-s commented Mar 17, 2021

I believe the underlying problem is the same as in #5765 and it should be fixed with #5850 when we have upstream fix rolled out.

@yury-s
Copy link
Member

yury-s commented Mar 17, 2021

@thernstig can you check that it works fine in Firefox and WebKit?

@thernstig
Copy link
Contributor Author

@yury-s I have the same problem in Firefox:

  pw:api attempting click action +1ms
  pw:api   waiting for element to be visible, enabled and stable +1ms
  pw:api   element is visible, enabled and stable +42ms
  pw:api   scrolling into view if needed +0ms
  pw:api   done scrolling +1ms
  pw:api   checking that element receives pointer events at (732.48,526) +0ms
  pw:api   <xxx-base-v0-button primary="" slot="bottom" id="apply-button"></xxx-base-v0-button> intercepts pointer events +2ms
  pw:api retrying click action, attempt #1 +0ms
  pw:api   waiting for element to be visible, enabled and stable +0ms
  pw:api   element is visible, enabled and stable +29ms
  pw:api   scrolling into view if needed +0ms
  pw:api   done scrolling +1ms
  pw:api   checking that element receives pointer events at (732.48,526) +0ms
  pw:api   <xxx-base-v0-button primary="" slot="bottom" id="apply-button"></xxx-base-v0-button> intercepts pointer events +2ms

(webkit gives me some other strange errors that I do not want to pollute this thread with)

@yury-s
Copy link
Member

yury-s commented Mar 18, 2021

@thernstig could you share source of the page where this is reproducible? I tried with a simple example below and it worked fine:

<template id="my-button-template">
  <button href="#" id="inner-button" onclick="clickCount++">Click me</button>
</template>
<my-button></my-button>
await page.click('my-button button');

@thernstig
Copy link
Contributor Author

@yury-s I am afraid I cannot, it is propertiary and messy in how it is written. Is there any simple way you can think of to get the DOM from Chrome dev tools to extract what you need? I suppose it depends exactly on how the bug shows itself so hard to know exactly what I need to supply.

Looking at the example you pasted, it looks different from what I posted in the picture. We use await page.click('#apply-button button'); with an ID, but maybe that is not important in how Playwright works here.

Looking at your example, we put the <button> in the shadow dom of <my-button>. Are you also doing that there?

@yury-s
Copy link
Member

yury-s commented Mar 18, 2021

with an ID, but maybe that is not important in how Playwright works here.

Matching by id vs tag name should no matter in this case.

Looking at your example, we put the in the shadow dom of . Are you also doing that there?

The button is written in the template in my case. I believe the bug has something to do with how elements are slotted into the template, so it'd be really helpful to see <template> node that defines <slot name="bottom"> and also the place where <div slot="bottom"> is inserted.

I will experiment more with different ways to slot it into template. If there is a chance you could share access to the page privately you can send instructions to the email in my profile, meanwhile I'll try to create a repro based on what you shared.

@thernstig
Copy link
Contributor Author

I will see what I can do, I understand this is very hard with my sparse information. If it is of any help, we use lit-html to define our tempates. Maybe this gives some context?

html`
      <xxx-base-v0-dialog
        .label="${this.actionText}"
        class="create-edit-user"
        ?show=${this.show}
      >
        <div slot="content">${this.renderFormRows()}</div>
        <div slot="bottom">
          <xxx-base-v0-button
            id="apply-button"
            primary
            .disabled="${this.applyDisabled}"
            slot="bottom"
            @click="${async () => {
              await this.checkValidationAll();
              if (this.applyDisabled) {
                return;
              }
              this.resetDropdownState();
              this.user = {};
              this.resetInputState();
              this.bubble('apply-click', this.getData());
            }}"
            >${this.actionText}</xxx-base-v0-button
          >
        </div>
      </xxx-base-v0-dialog>
    `

@thernstig
Copy link
Contributor Author

Aside: We are using lit-html extensively, to generate web components with shadow DOM where the <template> rendered by lit-html is always inserted into custom elements with shadow DOM. I have more problems with this than just this one that looks different than this, but I do not want to create more such issues right now as it is possible whatever we find here might solve those/some of those as well. Maybe it is possible to do some more extensive testing with shadow DOM, custom elements, slots, templates (and maybe even lit-html? but that should probably not be needed as it just renders HTLM templates in the end).

@thernstig
Copy link
Contributor Author

thernstig commented Mar 18, 2021

Maybe I can also add there is an actual mistake done in the coding in the the picture I attached. There are two slot=bottom attribute both on the <div> and later on the <xxx-base-v0-button>. But the outer element <xxx-base-v0-dialog> only has one such bottom slot name in it. Not sure if that coding mistake does anything here to confuse Playwright.

@yury-s
Copy link
Member

yury-s commented Mar 18, 2021

I tried mimic your example with this code but it works just fine, there must be something special about definition of <xxx-base-v0-dialog> and xxx-base-v0-button or perhaps their styles.

There are two slot=bottom attribute both on the <div> and later on the <xxx-base-v0-button>.

This shouldn't matter since <div slot="bottom"> is not a template.

@thernstig
Copy link
Contributor Author

thernstig commented Mar 18, 2021

@yury-s I promise that I will try to see if I can get you the definitions of those tomorrow. It's late night here now so need some sleep :) The fault is reproducible for me though, so I just need to give you enough info here to pin it down.

@thernstig
Copy link
Contributor Author

thernstig commented Mar 18, 2021

Although from your code I can see there is no shadow dom involved, which it is in my case. I would think it is the combination of that with the slots seen that could be problematic. <xxx-base-v0-button slot="bottom"> has a shadow-root with the <button> in it.

@yury-s
Copy link
Member

yury-s commented Mar 18, 2021

I promise that I will try to see if I can get you the definitions of those tomorrow. It's late night here now so need some sleep :)

Thank you and good night!

Although from your code I can see there is no shadow dom involved

Well, there is a couple of shadow roots but I'm not sure close they are to what you have in your code:
image

@thernstig
Copy link
Contributor Author

Some more info before I try a few other things, no idea if this helps. Also not sure I ever mentioned this, but clicking this works just fine:

// work just fine
await page.click('#apply-button');

// this does not work though
await page.click('#apply-button button');

Here is the important CSS in regards to display etc.

/* css for <xxx-base-v0-dialog> */
:host([show]) {
    display: flex;
    opacity: 1;
    flex: 0 0 auto;
}

:host {
    position: fixed;
    left: 0;
    top: 0;
    width: 100%;
    height: 100%;
    font-family: var(--font-main, 'Helvetica');
    text-rendering: optimizeLegibility;
    -webkit-font-smoothing: antialiased;
    display: none;
    align-items: center;
    justify-content: center;
    z-index: 100;
    background: var(--dialog-overlay, rgba(0, 0, 0, 0.3));
}

/* css for <xxx-base-v0-button> */
host {
    display: inline-block;
    text-rendering: optimizeLegibility;
    -webkit-font-smoothing: antialiased;
}

/* css for <button> */
button {
    display: flex;
    width: 100%;
    text-align: center;
    flex-direction: row;
    padding: 5px 12px 4px;
    border-radius: var(--btn-radius, 3px);
    outline: 0;
    font-size: var(--btn-font-size, 14px);
    color: var(--btn-secondary-text, #242424);
    background-color: var(--btn-secondary-background, transparent);
    border: 1px solid var(--btn-secondary-border, #242424);
    line-height: 1.5;
    box-sizing: border-box;
    background-image: var(--btn-secondary-gradient);
    font-family: var(--font-main, "Helvetica", "sans-serif");
}

Here are the LitComponent render functions:

For <xxx-base-v0-dialog>:

  render() {
    const bottom = this.fullscreen ? '' : html`<div class="dialog__bottom ${this.warning ? 'dialog__bottom--reverse' : ''}">
      ${this.noCancel ? '' : html`<xxx-base-v0-button class="cancel" @click="${this.hideDialog}">'Cancel'</xxx-base-v0-button>`}
      <slot name="bottom">
            </slot>
          </div>`;
    const right = this.fullscreen ? html`<div class="dialog__right">
                  <xxx-v0-icon class="cross" name="cross" size="14px" @click="${this.hideDialog}"></xxx-v0-icon>
                  </div>` : '';
    return html`
          <div class="dialog">
            <div class="dialog__top">
              <div class="dialog__title">${this.label}</div>
              ${right}
            </div>
            <div class="dialog__body">
              <slot name="content"></slot>
            </div>
            ${bottom}
          </div>`;
  }

Here is the render() for <xxx-base-v0-button> (which is put in the slot="bottom"):

render() {
return html`
      <button tabindex="0" class="${this._setClasses()}" ?disabled=${this.disabled}>
        ${this.reverse ? '' : this._withIcon()}
        <span class="button__label ${this._isIcon() ? 'with--icon' : ''}">
          <slot></slot>
        </span>
        ${this.reverse ? this._withIcon() : ''}
      </button>
      `;
}

@yury-s
Copy link
Member

yury-s commented Mar 19, 2021

Thanks for sharing this, I managed to reproduce it with this code, it doesn't fail in WebKit and Firefox though, only in Chromium so suspect it might be different issue. The bug that I reproduced is fixed with #5850

@thernstig
Copy link
Contributor Author

thernstig commented Mar 21, 2021

The same click fails for me in Firefox (as I tested that). How about I wait for #5850 and see if it helps once that is merged? I suspect it might be a while if it is pending an upstream change?

@yury-s
Copy link
Member

yury-s commented Mar 22, 2021

I hope it will get into the next dev version of Chromium which should be this or next week.

@thernstig
Copy link
Contributor Author

@yury-s better late than never, but I can confirm that it does work with the upgrade 😄

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 a pull request may close this issue.

4 participants