Skip to content

Fix rel attribute and add rel values#1072

Merged
ArneBab merged 1 commit into
hyphanet:nextfrom
torusrxxx:rel
Jun 28, 2025
Merged

Fix rel attribute and add rel values#1072
ArneBab merged 1 commit into
hyphanet:nextfrom
torusrxxx:rel

Conversation

@torusrxxx

Copy link
Copy Markdown
Contributor

Fix rel attribute and add rel values noopener,nofollow,noreferrer,external,tag
Also fixed code style related to adding items to HashSet.

@bertm bertm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This appears to introduce 3 changes, but does so in 1 commit:

  1. Change for to addAll - this change I'm happy to accept
  2. Fix something in the rel attribute handling - please explain in the commit message for this change what was broken and how it is fixed & make sure to have a test for the improvement
  3. Add new rel types, but not all of them - please explain explain in the PR description why we need exactly those to be added (and not some other ones that could also be supported)

In it's current state with the commented out code and the FIXME's unresolved, this PR is not ready to be merged.

I would assume change number 2 in the above list brings most value - if possible, please get that in a mergeable state without the parts that would require further discussion :)

@torusrxxx

Copy link
Copy Markdown
Contributor Author

Originally I just wanted to fix that doubled token.equalsIgnoreCase("stylesheet") check. But then I found more and more things to fix and the scope became unclear.
I now reverted to just coding style fixes. Thanks.

@ArneBab ArneBab merged commit 0dd6e99 into hyphanet:next Jun 28, 2025
1 check passed
@ArneBab

ArneBab commented Jun 28, 2025

Copy link
Copy Markdown
Contributor

Merged -- thank you!

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.

4 participants