Preview dialog - Dompurify parsing error with image shortcode

phiw13 · 2024-10-22 02:19:54

Textpattern 4.9-dev latest (and 4.9-beta1), PHP 8.3

Is there any reason why a shortcode with image code cannot be previewed with both “Sanitize” and “Tags” enabled?

Shortcode:

<txp:images id='<txp:yield name="id" />' break=''>
  <figure class="article-img" itemprop="image" itemscope itemtype="http://schema.org/ImageObject">
    <picture>
      <img src="<txp:image_url thumbnail='1' />" srcset="<txp:image_url /> 2x" alt="<txp:image_info type='alt' />" itemprop="url contentUrl"><txp:hide><txp:image_info type='w' />w</txp:hide>
    </picture>
    <figcaption itemprop="caption"><txp:image_info type="caption" escape="tidy, textile" /></figcaption>
  </figure>
</txp:images>

Steps to reproduce:

create a shortcode form with above content, named it my_images.
insert image(s) in article using above shortcode: <txp::my_images id="324,45,856" />
preview article in dialog widget
enable tag parsing, wait for rendering; the preview remains “greyed out”

Error in browser console (pointing to `textpattern.js:1415:109`):

Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'node.replaceWith(wrapNode).remove')

Note:

this is a very simplified shortcode stripping out as much as possible. Most of my other shortcode(s) seem to render correctly.
check OFF “Sanitize“, enable tag parsing works correctly

etc · 2024-10-22 08:16:08

Related to this, I guess. DOMPurify has introduced some breaking changes that I need to grok yet. Otherwise, we’d have to switch back to 3.1.5 I’m afraid. Thanks for the report.

phiw13 · 2024-10-22 08:27:10

etc wrote #338039:

Related to this, I guess. DOMPurify has introduced some breaking changes that I need to grok yet. Otherwise, we’d have to switch back to 3.1.5 I’m afraid. Thanks for the report.

Not sure, as those are related to the latest DOMPurify, I see the same issue with Textpattern 4.9-beta1.

Here is another shortcode that triggers the error (again reduce to the bare minimum):

<txp:file_download_list id='<txp:yield name="id" />'>
  <span itemscope itemtype="https://schema.org/DataDownload"><a href="<txp:file_download_link />" itemprop="url contentUrl"><span itemprop="name"><txp:file_download_name title /></span></a></span>
</txp:file_download_list>

(as used on the Sandspace page)

etc · 2024-10-22 09:09:59

You are probably right, it must be my silly attempt to visualise elements with sanitised attributes (like itemscope etc). BTW, how should it be done? For example, after clean-up

<span itemscope itemtype="https://schema.org/DataDownload">Download me!</span>

becomes

<span>Download me!</span>

But on Text preview they look the same. How should we warn the user that some (potentially harmful) attributes have been removed?

phiw13 · 2024-10-22 11:47:51

Conundrum…

On the one hand the content of a shortcode should be considered “safe” I think (approved by higher level people, can only be modified by people with the right privileges). The only way to send something naughty to the shortcode content is trough its attributes, which you can see when previewing the page in the dialog box (with only “sanitise” enabled)

<txp::shortcode_name attribute="value" other-attribute="value" />

On the other hand, the logic of tag parsing with “sanitise” enabled would suggest to neuter and flag those potentially problematic attributes. But the user reviewing the article expects the tag to be rendered.

Hmm. Thinking loud – is it possible to display both the rendered tag(sanitised) and its full source? That might be expensive performance wise, no idea. The reviewer can of course see the same by checking the source code tab of the dialog box – but for a long article that might not be all that easy.

phiw13 · 2024-10-28 02:52:45

Oleg, after your latest code changes over the WE, it works much better. The shortcodes are rendered as expected, no more browser console spam. TY.

Pat64 · 2024-10-28 07:03:27

phiw13 wrote #338079:

Oleg, after your latest code changes over the WE, it works much better. The shortcodes are rendered as expected, no more browser console spam. TY.

Much better.

There is only one behavior that could annoy redactors: links are opened in the same window (users lose the Textpattern interface)…

etc · 2024-10-28 18:05:31

Fair point, will change, thanks.

Another point I’m thinking of: do we need to see (faded out) representations of (normally invisible) sanitized elements (scripts, styles etc) in text preview? They can be annoying, but, on the other hand, partly explain why the orange warning is flashing.

phiw13 · 2024-10-28 23:52:34

etc wrote #338082:

Another point I’m thinking of: do we need to see (faded out) representations of (normally invisible) sanitized elements (scripts, styles etc) in text preview? They can be annoying, but, on the other hand, partly explain why the orange warning is flashing.

The point of that sanitised view is to show all potentially ~~dangerous~~ unsafe elements within the article body, so I would argue that yes, that is useful to editors.

Pat64 · 2024-10-31 02:50:24

Your latest commit is perfect!

Thank you lot. The target="_new" attribute injected to links within the preview article dialog makes sens with the “preview” link (eye icon).

Just an idea rising into my brain:

Could it be possible (as the availability of a custom.css for the admin interface) to offer us a css file into order to style some aspect of the dialog content? I think of short codes which could gain some specific’s layout. I tried to inject CSS into :host{} in order to interact with the Shadow DOM without any success…

etc · 2024-10-31 08:11:01

Pat64 wrote #338089:

Could it be possible (as the availability of a custom.css for the admin interface) to offer us a css file into order to style some aspect of the dialog content? I think of short codes which could gain some specific’s layout. I tried to inject CSS into :host{} in order to interact with the Shadow DOM without any success…

I’d love to, but injecting css rules into shadow dom is rather cumbersome (on purpose). Currently, the rules are stored in textpattern/preview.css, which you can edit as desired. The diagnostics would complain about modified files, though. Should we simply exclude it from checksums? Or should it be made part of the theme, or something else?

To enable styling shortcodes and txp tags, some extra markup is necessary, will think about it.

phiw13 · 2024-10-31 08:25:19

etc wrote #338090:

I’d love to, but injecting css rules into shadow dom is rather cumbersome (on purpose). Currently, the rules are stored in textpattern/preview.css, which you can edit as desired. The diagnostics would complain about modified files, though. Should we simply exclude it from checksums? Or should it be made part of the theme, or something else?

Simply excluding it from checksums would be enough, I think.

Textpattern CMS

Textpattern CMS support forum

#1 2024-10-22 02:19:54