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

Use Canonical URL for ogp_site_url #55

Open
ItayZiv opened this issue Feb 24, 2022 · 4 comments · May be fixed by #56
Open

Use Canonical URL for ogp_site_url #55

ItayZiv opened this issue Feb 24, 2022 · 4 comments · May be fixed by #56
Assignees
Labels
enhancement New feature or request

Comments

@ItayZiv
Copy link
Collaborator

ItayZiv commented Feb 24, 2022

Getting og:url automatically in RTD using html_baseurl doesn't work.
We probably want to use html_baseurl when ogp_sire_url isn't set regardless of if we are in an RTD environment. It might be better to recommend users to set html_baseurl and only set ogp_site_url if they need it to be different than the normal canonical URL.

@ItayZiv ItayZiv added the bug Something isn't working label Feb 24, 2022
@ItayZiv ItayZiv self-assigned this Feb 24, 2022
@TheTripleV
Copy link
Member

Why doesn't it work on RTD?

@ItayZiv
Copy link
Collaborator Author

ItayZiv commented Feb 25, 2022

If you look at the og tags of the website of this extension you will see the website tag is just the page name.
I'm quite sure this is because you can't modify the config object. My bad I missed something, still need to check why this happens tho.
I want to use this opportunity to use html_baseurl even not in RTD.'

Edit: Basically I think that we should have it set up so that html_baseurl is used unless ogp_site_url is set, if it is set use ogp_site_url. If both aren't configured, raise an exception.

@ItayZiv ItayZiv linked a pull request Feb 25, 2022 that will close this issue
@ItayZiv
Copy link
Collaborator Author

ItayZiv commented Feb 25, 2022

It seems that when I check the PR docs build (and the PR build on other PRs) it actually seems to get the correct URL (https://sphinxext-opengraph.readthedocs.io/en/latest/index.html).
Regardless I'd still like to do the aforementioned html_baseurl change, but not really sure why the latest docs don't work.

@ItayZiv
Copy link
Collaborator Author

ItayZiv commented Feb 25, 2022

Actually, it seems like these are two separate matters, the docs links to https://sphinxext-opengraph.readthedocs.io/en/latest/ as the default, yet the latest branch is not updated, while https://sphinxext-opengraph.readthedocs.io/en/stable is updated and also has og:url linked to https://sphinxext-opengraph.readthedocs.io/en/latest/index.html. I split this into #57, while this issue can remain for changing the extension to use html_baseurl outside of RTD (and refractoring the way this is currently done)

@ItayZiv ItayZiv added enhancement New feature or request and removed bug Something isn't working labels Feb 25, 2022
@ItayZiv ItayZiv changed the title Fix RTD Support / Use Canonical URL Use Canonical URL for ogp_site_url Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants