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

JS Error when pasting a post (shortcode related) #13527

Closed
youknowriad opened this issue Jan 26, 2019 · 4 comments · Fixed by #14365
Closed

JS Error when pasting a post (shortcode related) #13527

youknowriad opened this issue Jan 26, 2019 · 4 comments · Fixed by #14365
Labels
[Feature] Paste [Type] Bug An existing feature does not function as intended
Milestone

Comments

@youknowriad
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.

To Reproduce

Screenshots

capture d ecran 2019-01-26 a 5 24 59 pm

cc @iseulde @jorgefilipecosta

@wpaustria
Copy link

This issue came up during that discussion:
#11782
Here are the videos which belong to this issue:

@jorgefilipecosta
Copy link
Member

I'm having problems reproducing the JS error, (although the pasting of this content is very slow). @wpaustria what software are you copying the content from? Does the JS error also happen if the content is pasted using cmd+shift+v (or equivalent in your keyboard)? Thank you for all the help debugging this issue.

@youknowriad
Copy link
Contributor Author

@jorgefilipecosta I reproduced this error by copying the whole content, clicking the appender, and pasting.

@wpaustria
Copy link

@jorgefilipecosta As shown in the videos, I copied the content with CMD+C/CMD+V and with the context menu. Both ways don't work.
Software: Firefox 64 & latest Chrome & latest Chromium on Ubuntu 18.10.
I copied the content from a website, have a look:

https://gist.github.com/florianbrinkmann/4e9e4d23eaefb8b23484badddd848196

This is called the infamous "The Florian Brinkmann article".
Which shows copying is very slow. Before the latest changes copying this content to Gutenberg was impossible!

dmsnell added a commit that referenced this issue Mar 11, 2019
Fixes #13527
See original work in #12315

When pasting content which includes the `[caption]` shortcode we assume
that the content is well-formed (that there is not only an `<img />` in
there but also in the first position).

In this patch we fix the problem by removing the old code, which removed
the first `Element` node, and replaced it with code that removes the
first `IMG` node _if one is found_.

We're leaving other image nodes in place in case the caption contains
image nodes and we're not requiring that the `IMG` be the first child
of the caption shortcode in case people are wrapping the image in other
valid HTML like this...

```html
[caption]<a href="some.link"><img src="some.image"></a>[/caption]
```

See the new unit tests for a more complete specification of the
intended behaviors.

PR reviewed, debugged, and created by:
-> LANNISTER MOB <-
 - @codebykat
 - @dmsnell
 - @gwwar
 - @kwight
 - @mmtr
 - @obenland
 - @rodrigoi
 - @vindl
gziolo pushed a commit that referenced this issue Mar 18, 2019
* Fix: Pasting captions without an image fails

Fixes #13527
See original work in #12315

When pasting content which includes the `[caption]` shortcode we assume
that the content is well-formed (that there is not only an `<img />` in
there but also in the first position).

In this patch we fix the problem by removing the old code, which removed
the first `Element` node, and replaced it with code that removes the
first `IMG` node _if one is found_.

We're leaving other image nodes in place in case the caption contains
image nodes and we're not requiring that the `IMG` be the first child
of the caption shortcode in case people are wrapping the image in other
valid HTML like this...

```html
[caption]<a href="some.link"><img src="some.image"></a>[/caption]
```

See the new unit tests for a more complete specification of the
intended behaviors.

PR reviewed, debugged, and created by:
-> LANNISTER MOB <-
 - @codebykat
 - @dmsnell
 - @gwwar
 - @kwight
 - @mmtr
 - @obenland
 - @rodrigoi
 - @vindl

* Update stripFirstImage behavior to also remove matching topmost parent node
youknowriad pushed a commit that referenced this issue Mar 20, 2019
* Fix: Pasting captions without an image fails

Fixes #13527
See original work in #12315

When pasting content which includes the `[caption]` shortcode we assume
that the content is well-formed (that there is not only an `<img />` in
there but also in the first position).

In this patch we fix the problem by removing the old code, which removed
the first `Element` node, and replaced it with code that removes the
first `IMG` node _if one is found_.

We're leaving other image nodes in place in case the caption contains
image nodes and we're not requiring that the `IMG` be the first child
of the caption shortcode in case people are wrapping the image in other
valid HTML like this...

```html
[caption]<a href="some.link"><img src="some.image"></a>[/caption]
```

See the new unit tests for a more complete specification of the
intended behaviors.

PR reviewed, debugged, and created by:
-> LANNISTER MOB <-
 - @codebykat
 - @dmsnell
 - @gwwar
 - @kwight
 - @mmtr
 - @obenland
 - @rodrigoi
 - @vindl

* Update stripFirstImage behavior to also remove matching topmost parent node
youknowriad pushed a commit that referenced this issue Mar 20, 2019
* Fix: Pasting captions without an image fails

Fixes #13527
See original work in #12315

When pasting content which includes the `[caption]` shortcode we assume
that the content is well-formed (that there is not only an `<img />` in
there but also in the first position).

In this patch we fix the problem by removing the old code, which removed
the first `Element` node, and replaced it with code that removes the
first `IMG` node _if one is found_.

We're leaving other image nodes in place in case the caption contains
image nodes and we're not requiring that the `IMG` be the first child
of the caption shortcode in case people are wrapping the image in other
valid HTML like this...

```html
[caption]<a href="some.link"><img src="some.image"></a>[/caption]
```

See the new unit tests for a more complete specification of the
intended behaviors.

PR reviewed, debugged, and created by:
-> LANNISTER MOB <-
 - @codebykat
 - @dmsnell
 - @gwwar
 - @kwight
 - @mmtr
 - @obenland
 - @rodrigoi
 - @vindl

* Update stripFirstImage behavior to also remove matching topmost parent node
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Paste [Type] Bug An existing feature does not function as intended
Projects
None yet
3 participants