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

Incorrect og:url and twitter:url Meta Tags point to localhost:4321, causing Mixed Content Issues on p5.js website #523

Closed
drashyakuruwa opened this issue Sep 7, 2024 · 8 comments
Labels
Bug Something isn't working

Comments

@drashyakuruwa
Copy link

Most appropriate sections of the p5.js website?

Tutorials

What is your operating system?

Windows

Web browser and version

Version 128.0.6613.120 (Official Build) (64-bit)

Actual Behavior

When visiting pages such as https://p5js.org/tutorials/get-started/ and several tutorial pages, Mixed Content errors appear in the
browser console. These errors indicate that while the page is served over HTTPS, it attempts to load resources over HTTP, which is
blocked by modern browsers for security reasons.

image

When exploring the code it appears like the issue is in the file src/components/Head/index astro and the problematic lines are:

Line 42: meta property="og:url" content=(Astro.url.href)
Line 50: meta name="twitter:url" content={Astro.url.href)

In Astro, Astro.url does not exist by default. To dynamically generate the URL, you need to access the current page's URL using the
appropriate method or variable. Unfortunately, Astro.url.href is likely causing problems because it isn't a valid API in the Astro
framework.

Expected Behavior

Steps to reproduce

  1. Visit any tutorial page on the p5.js website (for example, (https://p5js.org/tutorials/get-started/)).
  2. Inspect the page's source code (right-click > View Page Source).
  3. Locate the tags with the name="twitter:url" and property="og:url" attribute.
  4. Observe that the URL is http://localhost:4321/tutorials/get-started/ instead of the correct live URL.

image

Would you like to work on the issue?

Yes, I can work on the issue, have traced the problematic line and I know the fix.

@drashyakuruwa drashyakuruwa added the Bug Something isn't working label Sep 7, 2024
@limzykenneth
Copy link
Member

Hi @drashyakuruwa thanks for identifying where the issue comes from but can you briefly describe what the proposed fix would be?

@drashyakuruwa
Copy link
Author

drashyakuruwa commented Sep 12, 2024

Hello @limzykenneth, the fix of this would be to use Astro.request.url instead, as we can get the raw URL directly as requested by the client using that:

<meta property="og:url" content={Astro.request.url} />
<meta name="twitter:url" content={Astro.request.url} />

this provides the exact URL string as seen in the browser, including the domain, path, and query parameters. So, in development, we will still see the localhost URL instead of the production URL, but when deployed, Astro.request.url would provide the actual live URL requested by the client.

@limzykenneth
Copy link
Member

@drashyakuruwa I don't think that will work either, Astro.url is constructed from Astro.request.url and so Astro.url.href should basically have the same value as Astro.request.url in default configuration, the other thing is that Astro does not have a run time, it is statically built and the URL values are determined at built time not runtime, which means that there is no way for Astro.request.url to know the actual live URL requested by the client.

The suggested solution in Astro's docs is to set the site configuration instead which will be populated for Astro.url at built time with the set value.

@drashyakuruwa
Copy link
Author

@limzykenneth Ohh, yes, this makes sense now! I misunderstood Astro's static nature. I see that adding site: 'https://p5js.org' in the astro.config.mjs file is the right approach. Using Astro.site, we can check if the URL matches the production URL and set a fallback for local development. Thank you for helping clarify that!

@limzykenneth
Copy link
Member

Adding site: 'https://p5js.org' should be all that is needed to fix this and I don't think we need to implement anything else. @drashyakuruwa If you'd like to create a PR that add that config you can go ahead. Thanks.

@drashyakuruwa
Copy link
Author

@limzykenneth yes yes, I would like to do it, I will create it, Thank you.

@drashyakuruwa
Copy link
Author

@limzykenneth There is already PR raised by Shibom for #547 , which has the fix I was going to do.

@limzykenneth
Copy link
Member

Thanks @drashyakuruwa. This is now fixed by #547

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants