Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.4k stars 1.98k forks source link

Editor: Embed block shows only link in public page #48072

Open Nic-Sevic opened 3 years ago

Nic-Sevic commented 3 years ago

Steps to reproduce

Duplicate, once with AT (Business) site and once with Prem site can see on https://aacm2345.blog/2020/12/06/embed-test/

  1. Starting with URL to embed: https://www.joemygod.com/2020/12/cultist-hurls-homophobic-slurs-at-lockdown-protest/
  2. Create new post and add embed block
  3. add link to embed and publish page
  4. view published page in new tab
  5. view published page in incog tab

What I expected

Embed should show as seen on live page when logged into account Should appear the same on Prem and Business site

What happened instead

Embed shows as link when viewed from incog window on AT site Shows correctly only in editor on Prem site, everywhere else link

Browser / OS version

Mac OSX 10 Chrome 86.0.4240.198

Screenshot / Video

Embed in editor on AT site:

AT embed

Embed in Preview on AT site:

embed preview

Embed on live site on AT site:

embed live at

Embed on live site incog on AT site:

embed no logged in

Context / Source

3539525-zen

user-report

sbathompson-he commented 3 years ago

A user reported this error on Zen - 3807272

serabi commented 3 years ago

Error reported in 28213642-hc

edwinho89 commented 3 years ago

Another report in 28213642-hc

Embeds are shown correctly when viewed on Safari. Embeds are shown as links when viewed on Chrome.

automattic-ian commented 3 years ago

Embeds are broken in chrome and Edge, (with no console errors) but show fine in safari and firefox. reported in 28317863-hc says it only started yesterday around 1PM CST.

jsnajdr commented 3 years ago

I see two kinds of errors when testing the joemygod.com embed.

The first issue is with lazy loading the iframe. The markup looks like:

<iframe loading="lazy" style="position: absolute; clip: rect(1px, 1px, 1px, 1px); src="...">

The iframe is lazy loaded, i.e., loaded only when the element is within the viewport. But the styles effectively hide the iframe (until it's loaded). The 1x1px clip is apparently not enough to convince Chrome that the frame is in the viewport and that it should be loaded. When I remove the styles in Devtools, the frame immediately loads.

The second issue is that the joemygod.com site uses the Cloudflare CDN. Sometimes the iframe load returns a 503 Service unavailable HTTP error, and the response has an X-Frame-Options: SAMEORIGIN header. Making it an invalid content for an iframe. That happens often in Firefox for me.

jsnajdr commented 3 years ago

@automattic-ian writes on a P2:

The [...] issue has been open for a while, but it seemed to really only get a large number of reports in the past 24 hours ish.

I don't know what originally broke the Atomic embeds in Dec 2020, but the recent surge clearly coincides with release of WordPress 5.7 which adds a new iframe lazy-loading feature:

Now it’s simple to let iframes lazy-load. By default, WordPress will add a loading="lazy" attribute to iframe tags when both width and height are specified.

This apparently breaks some WordPress.com embeds, because the wpcom embeds/render REST endpoint returns iframe markup with styles that hide the iframe until it's loaded. And loading="lazy" defers loading the iframe until it's visible ⚑

@simison Is there a wpcom team that would be best-suited for fixing this bug? Either by reworking the iframe loading logic, or by simply disabling the new lazy-load feature with the wp_iframe_tag_add_loading_attr filter.

simison commented 3 years ago

Good sleuthing Jarda! It sounds like just removing the styles returned from the API endpoint would do the trick?

@jeyip @deBhal could be able to have a look at this? Thanks!

jsnajdr commented 3 years ago

It sounds like just removing the styles returned from the API endpoint would do the trick?

The styles ensure that the iframe is not visible until it loads successfully. If we simply remove them, visitors might see ugly white rectangles instead of nice embeds. Before making such changes, let's check if it doesn't degrade the UX visually.

jsnajdr commented 3 years ago

I realized that this issue is actually two different bugs.

One of them is the lazy loading of iframes that started few days ago with WordPress 5.7.

The second one is the originally reported one, involving Chrome incognito mode. That's an issue with the Cloudflare CDN and has indeed something to do with 3rd party cookies (hi @josephscott)

The joemygod.com page uses the Cloudflare Challenge Passage feature to protect itself against DDoS attacks. When a protected page is accessed for the first time, it shows an interstitial page:

Screenshot 2021-03-13 at 13 34 29

That page does some checks and then sets a cf_clearance page cookie and proceeds to load the target page.

If your browser already has the cf_clearance cookie, the check is not done and the target page is returned outright.

There are two issues with the Cloudflare check page:

  1. The response has the X-Frame-Origin: SAMEORIGIN header and it cannot be displayed in an iframe. The DDoS check can never be done in an embed iframe. The embed has a chance to load only when you already visited the site before as first party and the cf_clearance cookie was set there.
  2. Even if you already have the cf_clearance cookie, it won't be sent if the browser has 3rd party cookies blocked. That's what Chrome Incognito mode does by default. Again, the embed will never load.

This is an issue with the Cloudflare Challenge Passage protection. It just doesn't work with iframe embeds. And I don't see any workaround that the WP.com site that does the embed could use.

@blackjackkent Do we support enabling the Cloudflare Challenge Passage (DDoS protection) on wpcom sites as part of our partnership? Or is it only the CDN and Analytics? If we have any technical contact at Cloudflare, this issue would be a good question to ask and report.

jeyip commented 3 years ago

@jsnajdr @deBhal I was planning to add a filter for wp_iframe_tag_add_loading_attr, but I noticed that the loading="lazy" attribute is only added to iframe embeds on atomic sites πŸ€”

Atomic Simple
Screen Shot 2021-03-14 at 7 21 02 PM Screen Shot 2021-03-14 at 7 20 58 PM

Technical Details

I went digging, and it looks like the wp_filter_content_tags calls wp_iframe_tag_add_loading_attr ( Link ). The wp_filter_content_tags filter, however, is never passed an iframe and subsequently executes an early return, even if we clearly have iframe embeds in the page or post content ( Link ). I'm wondering if there's another bug on top of everything we're already working on 😞 I might be missing something, so would love for others to chime in.

devNigel commented 3 years ago

Another report: 3825320-zen

Thelmachido commented 3 years ago

Another report probably related. The user is on an iPhone + Chrome and their embed is looking like this screenshot here instead of this screenshot .

3832486 - zen

kwight commented 3 years ago

I don't consider this high priority – the link is still functional, so while it's not optimal or intended, I'll drop this to normal.

donalirl commented 3 years ago

Another case of this in #29001406-hc. Embedding a post on a simple site renders a nice card, but the same post on an atomic site shows a nice card in the editor but just a plain hyperlinked text on the live site.

Follow-up in #3906861-zen when fixed.