Fix #242: keep id,media,title attributes on replaced link elements #243
Conversation
✅ Deploy Preview for anchor-position-wpt canceled.
|
✅ Deploy Preview for anchor-polyfill ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| const promise = new Promise((res) => { | ||
| link.onload = res; |
There was a problem hiding this comment.
I'm fine including disabled here without a test, but we'll need to adjust the promise since onload won't fire for a disabled link. Something e.g.:
| const promise = new Promise((res) => { | |
| link.onload = res; | |
| link.disabled = el.disabled; | |
| const promise = new Promise((res) => { | |
| if (link.disabled) { | |
| res(true); | |
| } else { | |
| link.onload = res; | |
| } |
There was a problem hiding this comment.
I was thinking about this earlier too. I wonder if it might be better to change fetchCSS to just ignore stylesheets with disabled (both link and style)
There was a problem hiding this comment.
Let's merge this, and could you open a new issue to consider what to do with disabled link/style elements? It would be a breaking change to ignore disabled stylesheets without a watcher of some sort, since removing the disabled attr would fetch the un-polyfilled styles.
jamesnw
left a comment
There was a problem hiding this comment.
I was going to recommend using el.getAttributes() to get a fuller list, but as you point out in the issue, there's going to need to be some more intentional thought put in on which ones we actually don't want to copy. I'm happy if we look into that on a subsequent PR.
I could not get the test to work with the
disabledattribute (jsdomdoes not implement that attribute) so I just left it out.