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

Embed Block: pasting URLs to the WordPress codex tries to embed, fails #4799

Closed
jasmussen opened this issue Feb 1, 2018 · 18 comments
Closed
Assignees
Labels
[Feature] Blocks Overall functionality of blocks [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@jasmussen
Copy link
Contributor

Steps to reproduce:

  • Paste a URL to the WordPress codex into a paragraph block, for example http://codex.wordpress.org/Using_Themes
  • Note that it tries to create an embed block, then fails

GIF:

pasting codex

@jasmussen jasmussen added the [Type] Bug An existing feature does not function as intended label Feb 1, 2018
@adamsilverstein
Copy link
Member

I don't think these are supposed to expand at all, when I checked in the classic editor they do not. I'm not certain what logic determines if the link you paste is treated as an oembed?

@bph
Copy link
Contributor

bph commented Feb 4, 2018

I ran into a similar issue since update to 2.1, consistently with many different website, not just WP Codex/ here is the screen recording: http://recordit.co/ZZDZpJ9gra
The page I tried to include: http://www.advancedwp.org/awp-gutenberg-interview-series/

Message in the Browser Console:
index.js?ver=1516911562:12 GET https://gutenbergtimes.com/wp-json/oembed/1.0/proxy?url=http%3A%2F%2Fwww.advancedwp.org%2Fawp-gutenberg-interview-series%2F&_wpnonce=c046f5548c 404 ()
value @ index.js?ver=1516911562:12
Le @ react-dom.min.3583f8be.js:92
invokeGuardedCallback @ react-dom.min.3583f8be.js:91
invokeGuardedCallbackAndCatchFirstError @ react-dom.min.3583f8be.js:91
cd @ react-dom.min.3583f8be.js:15
Me @ react-dom.min.3583f8be.js:94
af @ react-dom.min.3583f8be.js:94
da @ react-dom.min.3583f8be.js:16
Zb @ react-dom.min.3583f8be.js:17
pb @ react-dom.min.3583f8be.js:123
yf @ react-dom.min.3583f8be.js:34
batchedUpdates @ react-dom.min.3583f8be.js:169
cc @ react-dom.min.3583f8be.js:26
jc @ react-dom.min.3583f8be.js:35
13:12:53.002 index.js?ver=1516911562:12 Fetch failed loading: GET "https://gutenbergtimes.com/wp-json/oembed/1.0/proxy?url=http%3A%2F%2Fwww.advancedwp.org%2Fawp-gutenberg-interview-series%2F&_wpnonce=c046f5548c".

Let me know if you need more info to troubleshoot this. :-)

@bph
Copy link
Contributor

bph commented Feb 4, 2018

Even with a URL that works, like a tweet I still get messages in the Browser Console:
Screen Recording: http://recordit.co/gMQts2LRKp

One of the messages:
widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:12 Fetch finished loading: GET "https://syndication.twitter.com/settings".
r @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:12
t.exports @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:12
(anonymous) @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:12
n @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:5
(anonymous) @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:5
n @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:5
(anonymous) @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:5
(anonymous) @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:5
(anonymous) @ widget_iframe.83d5793f6ebbe2046330abda6016ae93.html?origin=https%3A%2F%2Fgutenbergtimes.com:12
13:41:17.171 widgets.js:8 Blocked form submission to 'https://syndication.twitter.com/i/jot' because the form's frame is sandboxed and the 'allow-forms' permission is not set.

Let me know what else you would need to troubleshoot this.

@jasmussen
Copy link
Contributor Author

CC: @notnownikki as I recall you working on embeds.

@notnownikki
Copy link
Member

Will take a look

@notnownikki
Copy link
Member

Ok, the simpler issue first - the messages you see in the console when you embed Twitter content is because we sandbox the embedded content so it cannot interfere with the editor. They're normal in this content.

The other issue is more difficult. Codex URLs cannot be embedded, but the way we deal with pasting URLs assumes that every URL you paste can be embedded. If they can't you end up with an Embed block in an error state.

I think there are a couple of ways we could solve this.

  1. We allow embed blocks to optionally transform back into paragraphs if the embed is not successful.

or

  1. We don't call createBlock directly in the transform, instead we attempt to fetch the embedded content using the embed API, and if it's successful we return a new embed block with the embedded content already there, otherwise we return the current node.

Option 2 is (probably?) more work, but would be a smoother experience for the user - the UI wouldn't jump about as embed blocks tried and failed to embed unsupported URLs and transformed back, as they would in option 1.

cc: @gziolo (as your name is next to the transform 😄 )

@gziolo
Copy link
Member

gziolo commented Feb 5, 2018

cc: @gziolo (as your name is next to the transform 😄 )

Unfortunately, this seems to be an unlucky coincidence. I can't help much here as I didn't work with transforms :(

@jasmussen
Copy link
Contributor Author

Thanks a bunch for investigating Nikki, I assumed it was a conflict with the WordPress post embed feature since "wordpress.org" was in the URL.

CC: @iseulde as I believe you've worked on pasting, perhaps you can shed light?

@notnownikki
Copy link
Member

haha ok 😄

Then I'll pick this up and go with option 2 then!

@notnownikki notnownikki self-assigned this Feb 5, 2018
@ellatrix
Copy link
Member

ellatrix commented Feb 5, 2018

We are attempting to embed any link and fall back to the error. The plan was to show something for any link when pasting on its own line, through parsing the Open Graph tags (just like Twitter, Facebook etc.) I have a local WIP branch on this actually. I should push it some time. :)

@bph
Copy link
Contributor

bph commented Feb 6, 2018

@notnownikki It's not only the WordPress Codex web pages that throw the error. The example I posted earlier doesn't work. http://www.advancedwp.org/awp-gutenberg-interview-series/ Although if I wanted to share the link on Facebook it pulls in all the relevant information from the OpenGraph - So that idea would be really worth pursuing. :-) Let me know how can help testing/ troubleshooting this.

@notnownikki
Copy link
Member

@bph thank you! Yes, it's any url pasted on its own line where oEmbed doesn't support it. I'll be taking a close look at the OpenGraph WIP from @iseulde 😄

@designsimply designsimply added the [Feature] Blocks Overall functionality of blocks label Jul 3, 2018
@designsimply designsimply changed the title Pasting URLs to the WordPress codex tries to embed, fails Embed Block: pasting URLs to the WordPress codex tries to embed, fails Jul 3, 2018
@designsimply
Copy link
Member

Closing because I tested with WordPress 4.9.6 and Gutenberg 3.1.1 and was not able to see the bug happen where an embed block fails with an error for links such as http://codex.wordpress.org/Using_Themes or http://www.advancedwp.org/awp-gutenberg-interview-series/ — in cases where a non-embeddable link is used, the embed block is displays them as simple links.

screen shot 2018-07-03 at tue jul 3 3 59 20 pm
Seen at http://alittletestblog.com/wp-admin/post.php?post=13869&action=edit running WordPress 4.9.6 and Gutenberg 3.1.1 using Firefox 61.0 on macOS 10.13.5.

@notnownikki
Copy link
Member

Updating...

@iseulde asked me to look at a solution for this that generated a generic preview for pasted URLs that could not be embedded, it's a bit old now and needs rebasing, but it's at #7350 and if you paste an unsupported URL you get an editable paragraph block with the URL in it as a link, and a preview block generated from OpenGraph data. Could do with some eyes on it if we want to target it for inclusion in a future release? :)

@aaronjorbin
Copy link
Member

I've put this in the 5.0 milestone since there is an existing PR, but if this isn't going to be done for then, we should move it to future.

@aaronjorbin aaronjorbin added this to the WordPress 5.0 milestone Oct 8, 2018
@mtias mtias added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Oct 12, 2018
@notnownikki
Copy link
Member

The existing PR for this depends on a new OpenGraph API endpoint, so I'd suggest this goes into a future release if that's the approach we want to take.

I'd like to see unembeddable pasted URLs just end up in a paragraph block though, and that seems like a more realistic thing to get into 5.0

@notnownikki
Copy link
Member

#11808 has a fix for this, and takes into account that we might want an OpenGraph block option in the future, so it's not tightly coupled to the paragraph block.

@mtias mtias added the [Status] In Progress Tracking issues with work in progress label Nov 15, 2018
@gziolo
Copy link
Member

gziolo commented Nov 22, 2018

Fixed with #12095 which solves it as follows:

On the UI that tells the user that the URL could not be embedded, this adds a button to convert the URL to a paragraph block with a link, and a button to attempt the embedding again.

@gziolo gziolo closed this as completed Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

9 participants