Skip to content

Remove srcset attribute from list of "link" attrs#393

Merged
mre merged 4 commits intolycheeverse:masterfrom
untitaker:srcset-no-attr
Nov 16, 2021
Merged

Remove srcset attribute from list of "link" attrs#393
mre merged 4 commits intolycheeverse:masterfrom
untitaker:srcset-no-attr

Conversation

@untitaker
Copy link
Copy Markdown
Collaborator

Fix #390

@mre
Copy link
Copy Markdown
Member

mre commented Nov 15, 2021

Oh yeah that's it! Can we add a test as well?

@untitaker
Copy link
Copy Markdown
Collaborator Author

@mre what kind of test are you looking for and where would I put one? I think a unittest is not very useful.

@mre
Copy link
Copy Markdown
Member

mre commented Nov 16, 2021

I've added a test to the branch we can work with. It's a very simple thing in the extractor. It's failing right now as it will not detect the links from the srcSet. Is that how we expected it to work? Probably we should change it so that it also finds the remaining links right?

@mre
Copy link
Copy Markdown
Member

mre commented Nov 16, 2021

Just pushed to your branch, hope that's fine. 😅

@mre
Copy link
Copy Markdown
Member

mre commented Nov 16, 2021

I think supporting the links from srcSet will be tricky. It is detected as one string and passed to linkify. :/

@mre
Copy link
Copy Markdown
Member

mre commented Nov 16, 2021

I've disabled them for now. From my side this is good to go. It's not perfect but it's definitely an improvement. @untitaker what do you say?

@untitaker
Copy link
Copy Markdown
Collaborator Author

untitaker commented Nov 16, 2021 via email

@untitaker
Copy link
Copy Markdown
Collaborator Author

untitaker commented Nov 16, 2021 via email

@mre
Copy link
Copy Markdown
Member

mre commented Nov 16, 2021

Nicely done. Thanks! 🚀

@mre mre merged commit d3ed133 into lycheeverse:master Nov 16, 2021
@untitaker untitaker deleted the srcset-no-attr branch November 18, 2021 18:34
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.

Lychee treats srcSet attribute as single URL

2 participants