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

Trac message not escaped properly #29

Open
dd32 opened this issue May 10, 2021 · 4 comments
Open

Trac message not escaped properly #29

dd32 opened this issue May 10, 2021 · 4 comments

Comments

@dd32
Copy link
Member

dd32 commented May 10, 2021

The message sent to trac is not escaped properly, for example:
https://themes.trac.wordpress.org/ticket/99043#comment:2

Screen Shot 2021-05-10 at 11 38 45 am

  1. Path to theme is wp-content/themes/test-theme/ could probably just strip that out and leave it as includes/template-parts.php:413
  2. WP_Hook->do_action should be displayed WP_Hook->do_action
  3. #1 should not be linked, this can be escaped by prefixing # with !

Another option instead of escaping would be for the details to be code blocked or inserted within a html block, which takes care of 2/3 above, but the content would need to be run through nl2br() to add the <br> tags for newlines.

Examples of those approaches:
Screen Shot 2021-05-10 at 11 43 23 am

@StevenDufresne
Copy link
Contributor

I originally went with an HTML approach but had to abandon it. I'm cursing myself for not documenting why. I think it was related to overflow... results ended up being hidden outside of the comments because of long continuous strings, like paths, and misinterpreted whitespace characters?

@dd32
Copy link
Member Author

dd32 commented May 10, 2021

Hmm.. Using a code block would result in it overflowing horizontally and being hidden, html blocks seem to work though.

Links were not automatically linked when using a html block though, but something like this appears to work:

-----------
{{{
#!html
<?php echo nl2br( make_clickable( $runner_output ) ); ?>
}}}

@StevenDufresne
Copy link
Contributor

Fortunately I said "I think" so I can't be held liable :). I'll look through the commit history and add my findings to this thread.

@StevenDufresne
Copy link
Contributor

The change was made in .org before the initial commit so I can't find the reason.

It's worth trying again with fresh eyes to confirm the approach. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants