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

Pasting inline shortcodes produces separate block #3806

Closed
aduth opened this Issue Dec 4, 2017 · 13 comments

Comments

Projects
None yet
5 participants
@aduth
Copy link
Member

aduth commented Dec 4, 2017

Issue Overview

When pasting text which includes an inline shortcode, three blocks are produced: a paragraph (text before shortcode), shortcode, and paragraph block (text after shortcode).

Steps to Reproduce (for bugs)

  1. Navigate to Posts > Add New
  2. Click into "Write your story" to create a new default block
  3. Paste text including an inline shortcode
    • Example: "Due to conflicting travel arrangements of some participants following this past weekend's WordCamp US, we will be canceling the JavaScript Chat for this week. The next meeting will resume as regularly scheduled on [time]Tuesday, December 12, 2017 at 14:00 UTC[/time]."

Expected Behavior

Since the original text was intended to be contained within a single paragraph, I would expect the inline shortcode to be respected.

Current Behavior

Excessive blocks are created.

Screenshots / Video

Inline shortcode paste

@mcsf mcsf self-assigned this Dec 5, 2017

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Dec 6, 2017

@aduth: thanks, this is a pain point and good to discuss. Right now the Shortcode block is the only way to get around the limitation presented in #3610:

The main benefit of this is that, within the Shortcode block, the code will be properly handled and never mishandled. Notably, this fixes the case where a shortcode sitting in a Paragraph block will be picked up with its double quotes rendered as ", thus breaking parsing of a shortcode's attributes.

Technical considerations if we want support for inline shortcodes:

  1. The Shortcode block's save method returns a plain string corresponding to the provided shortcode. The Gutenberg serializer handles plain strings differently, outputting them directly. Hence, what is provided in the Shortcode block's input field is what gets saved to post_content, verbatim.

  2. However, any content-full block, such as Paragraph, will have its save method return an element tree. These are ultimately serialized using React's renderToStaticMarkup, which unabashedly escapes the usual suspects, including " → ". There doesn't seem to be any mechanism offered through React's essential interface to change this.

  3. I don't know any good way to get around 2) aside from horribly, horribly traversing all element trees before serialization and dangerously messing with their HTML. See this proof-of-concept jsfiddle that retrofits react-dom's escapeTextForBrowser and also kills the Web, but does the job.

Any counterpoints that can get me out of this rabbit hole are welcome. 😁

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Dec 6, 2017

A similar paste issue, though not focused on shortcodes, up for review: #3823

@aduth

This comment has been minimized.

Copy link
Member Author

aduth commented Dec 11, 2017

Yeah, that's a tough one. I find that, at least for serialization, React's sanitization works against us more than it helps, particularly since we already have sanitization occurring server-side to post content (citation needed). Part of the idea in #2463 was to give us more control over how the tree was serialized, perhaps enabling us to skip the escaping. I'm generally not a fan that we have a "magic escape hatch" behavior that tests the return value being a string and allows it to inserted verbatim, and would like this to be supported as part of a single consistent return value handling.

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 4, 2018

@kmgalanakis, the experimental #4049 fixes this issue for me. Does it for you too? (edit: If so, I'll add the current issue as something that 4049 fixes.)

@kmgalanakis

This comment has been minimized.

Copy link

kmgalanakis commented Jan 4, 2018

@mcsf unless I'm doing something really wrong, this doesn' solve the issue for me. When pasting an inline shortcode inside the content of a paragraph, the shortcode is transformed into a separate shortcode block.

shortcode

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 4, 2018

Oh, apologies, @kmgalanakis, I didn't explain properly. This fixes the issue when typing in a shortcode (that's meant to sit inline). Once that's fixed, we'd have to prevent block conversion to happen for inline content.

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 4, 2018

More detailed: if I append [gallery ids="9,10,11"] at the end of a line of text in a Paragraph, then on the front-end the gallery will be properly rendered. Without this patch, the serialization would yield [gallery ids="9,10,11"], breaking ID parsing on the front-end, leading to the rendering of an incomplete gallery.

@kmgalanakis

This comment has been minimized.

Copy link

kmgalanakis commented Jan 5, 2018

@mcsf It seems that #4049 fixes the issue with the HTML entity transformation of the argument of the inline shortcodes. The inline shortcodes I've tested, play nice with this branch.

When do you think it will be merged?

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 12, 2018

Hey, @kmgalanakis. Estimating is tricky. I expect 4049 to be one of the things we'll focus on over the next releases, but we can't ship that one until it's really solid.

Edit: fixed in #5897

@bobbingwide

This comment has been minimized.

Copy link
Contributor

bobbingwide commented Apr 7, 2018

Does this Issue also cover the "Convert to blocks" action?

In a set of approx 200 shortcodes I have the following

Shortcode type Percentage
Block 75%
Inline 20%
Mixed 5%

For Mixed shortcodes the output varies depending on parameters.
e.g. [bw_plug oik-block] would produce an inline link to a plugin. `[bw_plug oik-block table=y] would produce a table showing latest information about the plugin.

I read somewhere that the transform to shortcode logic could be filtered by a plugin, but I don't know how to do it.

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Apr 17, 2018

Now that #5897 has been merged, solving issues around serializing entities like quotes in shortcodes, I think we can address this one by simply not converting shortcodes to blocks when they are inline. In other words, pasting:

Hello, how do you do?

[time]Tuesday, December 12, 2017 at 14:00 UTC[/time]

yields:

<!-- wp:paragraph -->
<p>Hello, how do you do?</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[time]Tuesday, December 12, 2017 at 14:00 UTC[/time]
<!-- /wp:shortcode -->

but pasting:

Hello, it is now [time]Tuesday, December 12, 2017 at 14:00 UTC[/time].

yields:

<!-- wp:paragraph -->
<p>Hello, it is now [time]Tuesday, December 12, 2017 at 14:00 UTC[/time].</p>
<!-- /wp:paragraph -->

cc @iseulde

@mcsf mcsf referenced this issue Apr 17, 2018

Merged

Add paste schema (fix various issues, simplify) #5966

3 of 3 tasks complete

@mcsf mcsf added this to the 2.8 milestone Apr 18, 2018

@ellatrix

This comment has been minimized.

Copy link
Member

ellatrix commented Apr 18, 2018

@mcsf Sure we could do this. How do we detect if they're inline? Check if there's a line break before? Or \n\s* maybe. That would break if all content is on a single line but I guess that never happens. At least it would be good to be bit more conservative, we can then make it more flexible if needed.

@bobbingwide

This comment has been minimized.

Copy link
Contributor

bobbingwide commented Apr 19, 2018

There are potentially multiple ways of detecting inline shortcodes.

  1. Shortcode appears within an HTML tag <p>I like [wp]. Do you?</p>
    Result: One paragraph block.
  2. Shortcode appears in untagged text.
I like [wp]. Do you?

Yes, but autop’s a battle sometimes.

Result: Two paragraph blocks.

  1. Shortcode defines itself as inline.

  2. Shortcode expansion doesn’t lead to the creation of block tags.

In my experience block or mixed shortcodes normally appear on separate lines

<h3>Contact</h3>
[bw_email]
[bw_telephone]
[bw_address]

When shortcodes are used within a whole block of shortcodes

[div class=w50p0] I like [wp] [ediv] [div class=w50p0] Ditto [ediv]

then the block conversion routine can’t infer much, and would probably produce the best result by bunging it all into one shortcode block.

In examples like this it’s likely that the shortcodes have been concatenation into one line simply to avoid issues with unwanted <br />s.

Obviously, what’s missing is the ability to define the each shortcode’s behaviour:

  • inline / block / mixed
  • requires end tag
  • converts into a block

IMO WordPress core’s code is out of touch with how shortcodes are being used in the wild.
The new editor should either respect existing content and undocumented conventions or provide well documented extensibility hooks to enable plugin/theme shortcode/block developers to help with the conversion. It’s got to be an improvement, not a degradation.

@ellatrix ellatrix referenced this issue Apr 20, 2018

Merged

Raw handling: skip inline shortcodes #6329

0 of 4 tasks complete

@mcsf mcsf closed this in #6329 Apr 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.