Skip to content

fix: ignore stylesheets fetching when is a inline style#4138

Merged
straker merged 3 commits intodequelabs:developfrom
EduSantosBrito:develop
Sep 5, 2023
Merged

fix: ignore stylesheets fetching when is a inline style#4138
straker merged 3 commits intodequelabs:developfrom
EduSantosBrito:develop

Conversation

@EduSantosBrito
Copy link
Copy Markdown
Contributor

Closes: #4137

This fixes the issue when there's a inline style tag inside the shadowRoot. Since the inline style doesn't have a href, there's no need to fetch it. This is usual on Lit components.

@EduSantosBrito EduSantosBrito requested a review from a team as a code owner August 25, 2023 11:51
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Aug 25, 2023

CLA assistant check
All committers have signed the CLA.

@straker
Copy link
Copy Markdown
Contributor

straker commented Aug 28, 2023

Thanks for the pull request. I would think that the filter method filerStyleAndLinkAttributesInDocumentFragment would catch the inline style tag and filter it out of the loop, thus never getting into this problem. A simple example shows that this is what happens. Do you know why in your case the filter is letting the inline style tag through?

<div id="shadow"></div>
<script>
    const root = shadow.attachShadow({ mode: 'open' });
    root.innerHTML = `
      <!-- this is filtered out of the getStylesheetsFromDocumentFragment loop -->
      <style>
      a { color: red }
      </style>
      <a href="#">hello world</a>
  `;
</script>

@WilcoFiers
Copy link
Copy Markdown
Contributor

@EduSantosBrito Quick ping, have you had a chance to look at Steve's comment?

@EduSantosBrito
Copy link
Copy Markdown
Contributor Author

Hey! I have time this week to look at it. I was on holiday last week hahaha

@EduSantosBrito
Copy link
Copy Markdown
Contributor Author

Ok, after debugging for some hours I found that document.implementation.createHTMLDocument breaks HTMLStyleElement.sheet. There's some issues opened related to that jsdom/jsdom#3179

This makes sense why the browser using selenium-webdriver was working and the test using jsdom wasn't.

Quick ping @WilcoFiers @straker

@straker
Copy link
Copy Markdown
Contributor

straker commented Sep 5, 2023

That makes sense. Thanks for the looking into that for us.

Copy link
Copy Markdown
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Just updating the comment to reflect the investigations on when and why this is needed.

EduSantosBrito and others added 2 commits September 5, 2023 16:26
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
@straker
Copy link
Copy Markdown
Contributor

straker commented Sep 5, 2023

Approved for security.

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.

Shadow Root with inline style fails to fetch cssom

4 participants