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

*marked* issues with HTML #1775

Closed
Martii opened this issue Nov 9, 2020 · 2 comments · Fixed by #2016
Closed

*marked* issues with HTML #1775

Martii opened this issue Nov 9, 2020 · 2 comments · Fixed by #2016
Labels
bug You've guessed it... this means a bug is reported. dependency issue Hmmph! A dependency issue

Comments

@Martii
Copy link
Member

Martii commented Nov 9, 2020

See on OUJS pro at Markdown doesn't render kbd element correctly

Affected tag(s):

  • kbd
  • strong
  • code
  • ... (Probably more)

Generating text element after tags i.e. <tag></tag>Text instead of <tag>Text</tag>

Ref(s):

@Martii Martii added bug You've guessed it... this means a bug is reported. dependency issue Hmmph! A dependency issue labels Nov 9, 2020
@Martii
Copy link
Member Author

Martii commented Aug 18, 2021

Tables are broken in marked@3.0.0 atm.


Work-around available at markedjs/marked#2217 (comment)

Martii added a commit to Martii/OpenUserJS.org that referenced this issue May 24, 2022
* *marked* seems to have finally fixed tables... seems 1 to 1... still has issues with html tags per OpenUserJS#1775 via markedjs/marked#2217 and markedjs/marked#2471
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Jun 24, 2022
* Previews seem to be severely busted in v4.x ... will have to decide if a rollback or other change is needed, or if a different dep needs to be used [*markdown-it](https://www.npmjs.com/package/markdown-it) for example. I do prefer regex speed in *markdown* but a lot of errors.

Post OpenUserJS#1957 and may apply to OpenUserJS#1775

Note(s):
* `marked.min.js` is made by https://github.com/markedjs/marked/blob/078352f2198f9a2359b9c66286007202be919be1/Makefile#L3 as referenced in https://github.com/markedjs/marked/blob/078352f2198f9a2359b9c66286007202be919be1/README.md#usage
Martii added a commit that referenced this issue Jun 24, 2022
* Previews seem to be severely busted in v4.x ... will have to decide if a rollback or other change is needed, or if a different dep needs to be used [*markdown-it](https://www.npmjs.com/package/markdown-it) for example. I do prefer regex speed in *markdown* but a lot of errors.

Post #1957 and may apply to #1775

Note(s):
* `marked.min.js` is made by https://github.com/markedjs/marked/blob/078352f2198f9a2359b9c66286007202be919be1/Makefile#L3 as referenced in https://github.com/markedjs/marked/blob/078352f2198f9a2359b9c66286007202be919be1/README.md#usage

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Nov 9, 2023
* walkTokens in *marked* is not any better because `<kbd>` + `string` + `</kbd>` still happen individually... more maintenance needed.
* tokenizer in *marked* could work as long as the default handler wouldn't split the input but untested at the moment and I suspect it would split. Also even more to maintain.
* Only applying this workaround to `html` type input with single tags only for now. Nested is unsupported.
* Not closing atm because this is extremely hacky and it may not cover all instances since user-content can be anything.
* Next *marked* will be breaking in multiple places and requires yet another dependency to continue to work with highlighting at minimum.
* Alternative sanitizers, that may or may not auto close tags are either built on current, or have DOM only prerequisites/hacky Server implementation.
* Missing semicolon.

NOTE(s):
* `blockquote` renderer appears to never be called but haven't typed everything there is possible into user-content. ;)
* `innerHTML` *possibly* auto-closes just like GM_setStyle lib does with CSS in the DOM i.e. validation before execution... a form of sanitizing... currently untested but assumed for future ease-of maintainability.

Applies to OpenUserJS#1775
Martii added a commit that referenced this issue Nov 9, 2023
* walkTokens in *marked* is not any better because `<kbd>` + `string` + `</kbd>` still happen individually... more maintenance needed.
* tokenizer in *marked* could work as long as the default handler wouldn't split the input but untested at the moment and I suspect it would split. Also even more to maintain.
* Only applying this workaround to `html` type input with single tags only for now. Nested is unsupported.
* Not closing atm because this is extremely hacky and it may not cover all instances since user-content can be anything.
* Next *marked* will be breaking in multiple places and requires yet another dependency to continue to work with highlighting at minimum.
* Alternative sanitizers, that may or may not auto close tags are either built on current, or have DOM only prerequisites/hacky Server implementation.
* Missing semicolon.

NOTE(s):
* `blockquote` renderer appears to never be called but haven't typed everything there is possible into user-content. ;)
* `innerHTML` *possibly* auto-closes just like GM_setStyle lib does with CSS in the DOM i.e. validation before execution... a form of sanitizing... currently untested but assumed for future ease-of maintainability.

Applies to #1775

Auto-merge
@Martii
Copy link
Member Author

Martii commented Nov 9, 2023

Btw....

    if (aType === 'html') {
      arguments[1] = !!'block';  // This is force implementing marked#2768 addition... doesn't appear to work because it's already split e.g. <kbd> + string + </kbd>
    }
    // Sanitize first to close any tags
    var sanitized = sanitize(marked.Renderer.prototype[aType].apply(renderer, arguments));

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Nov 12, 2023
* Additional check just keeps it cleaner but the browser could change how it handles an isolated close tag... so just don't create it if it's blocked with a pre block test... plus a few bytes less out. Only need to do this on additions. `.replace` on empty string doesn't add nor on another tag such as `a` which can show up here.

Post OpenUserJS#2014 and applies to OpenUserJS#1775
Martii added a commit that referenced this issue Nov 12, 2023
* Additional check just keeps it cleaner but the browser could change how it handles an isolated close tag... so just don't create it if it's blocked with a pre block test... plus a few bytes less out. Only need to do this on additions. `.replace` on empty string doesn't add nor on another tag such as `a` which can show up here.

Post #2014 and applies to #1775

Auto-merge
@Martii Martii self-assigned this Nov 13, 2023
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Nov 13, 2023
* Sanitizer will still standardize on lowercase atm.
* Some optimize/minimize conditionals.
* Be specific on what/where to replace with open tags... not a huge deal atm but extra-cautious may be prudent.
* Restore some styling

Post OpenUserJS#2014 OpenUserJS#2015 and closes OpenUserJS#1775
Martii added a commit that referenced this issue Nov 13, 2023
* Sanitizer will still standardize on lowercase atm.
* Some optimize/minimize conditionals.
* Be specific on what/where to replace with open tags... not a huge deal atm but extra-cautious may be prudent.
* Restore some styling

Post #2014 #2015 and closes #1775

Auto-merge
@Martii Martii removed their assignment Nov 13, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug You've guessed it... this means a bug is reported. dependency issue Hmmph! A dependency issue
Development

Successfully merging a pull request may close this issue.

1 participant