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

Fix replacing some lazy-loaded widgets #2994

Merged
merged 5 commits into from
Jul 17, 2024
Merged

Conversation

ghostwords
Copy link
Member

@ghostwords ghostwords commented Jul 8, 2024

Fixes #2989. Follows up on #2852.

Also fixes replacing Embedly frame loaded (nested) surrogate JS API-powered widgets:

Lazy loaded example that still doesn't work:

Surrogate JS API-powered example that no longer works (unrelated to this PR; a "loading" overlay hides our placeholder):

@lenacohen
Copy link
Contributor

I tested the Embedly link and the links in #2989. The fix worked for everything except the YouTube player on https://www.vox.com/22362894/which-covid-vaccine-is-better-moderna-vs-pfizer-video

Also, while testing non-lazy-loaded widget pages, I noticed we might need to yellowlist sf16-website-login.neutral.ttwstatic.com to get TikTok widget replacements working on certain pages. Ex: https://goatagency.com/blog/influencer-marketing/super-bowl-ads/

@ghostwords
Copy link
Member Author

ghostwords commented Jul 12, 2024

I tested the Embedly link and the links in #2989. The fix worked for everything except the YouTube player on https://www.vox.com/22362894/which-covid-vaccine-is-better-moderna-vs-pfizer-video

OK, it's not a lazy load situation, but a YouTube API widget nested inside an iframe. The following patch fixes it but it would be better if we could instead allow surrogate API messages from all first party (according to our MDFP definitions) frames:

diff --git a/src/js/contentscripts/utils.js b/src/js/contentscripts/utils.js
index 8607f71a..0cc96e11 100644
--- a/src/js/contentscripts/utils.js
+++ b/src/js/contentscripts/utils.js
@@ -66,7 +66,8 @@ window.FRAME_URL = getFrameUrl();
 // investigate implications of third-party scripts in nested frames
 // generating pbSurrogateMessage events
 if (window.top != window) {
-  if (!window.FRAME_URL.startsWith('https://cdn.embedly.com/')) {
+  if (!window.FRAME_URL.startsWith('https://cdn.embedly.com/') &&
+    !window.FRAME_URL.startsWith('https://volume.vox-cdn.com/')) {
     return;
   }
 }

@ghostwords ghostwords added the widgets Click-to-activate placeholders for blocked but potentially useful social buttons/widgets label Jul 12, 2024
@lenacohen
Copy link
Contributor

lenacohen commented Jul 15, 2024

I tested the Embedly link and the links in #2989. The fix worked for everything except the YouTube player on https://www.vox.com/22362894/which-covid-vaccine-is-better-moderna-vs-pfizer-video

OK, it's not a lazy load situation, but a YouTube API widget nested inside an iframe. The following patch fixes it but it would be better if we could instead allow surrogate API messages from all first party (according to our MDFP definitions) frames:

...

Makes sense! Do we have already have an issue opened for this?

For messages from top level frames, and from nested frames
that are trivially first party (same scheme and host) to the tab.
If a nested frame is CNAME aliased to a first party hostname,
then let's accept widget surrogate messages as that frame
was opted into the site's infrastructure by the publisher.
@ghostwords ghostwords merged commit c4fedb3 into master Jul 17, 2024
2 checks passed
@ghostwords ghostwords deleted the lazy-loaded-widgets branch July 17, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
widgets Click-to-activate placeholders for blocked but potentially useful social buttons/widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lazy loaded widgets do not get replaced
2 participants