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 URLs #1883

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Use canonical URLs #1883

merged 1 commit into from
Jul 10, 2023

Conversation

mminer237
Copy link
Contributor

Fixes #1418

This links each community, user profile, and post to the original instance it was made on as canonical. It also links comments to their posts as canonical, since that seems recommended and that's what reddit does. The only pages with query parameters included in the canonical URL are search pages since those are pretty inherent to the page's content and since that seems to be what other sites do.

Comment on lines +470 to +471
this.context.router.route.match.url +
this.context.router.route.location.search
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought url already contains the query params?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have thought so from the name, but apparently not:

console.log(this.context.router.route);
{
  "location": {
    "pathname": "/search",
    "search": "?q=test&type=All&listingType=All&page=1&sort=TopAll",
    "hash": "",
    "state": null,
    "key": "1hxl4e7m"
  },
  "match": {
    "isExact": true,
    "params": {},
    "path": "/search",
    "url": "/search"
  }
}

A separate change might want to change the OpenGraph URLs some places as it indeed always links without the query parameters.

Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking care of this.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dessalines dessalines merged commit 53c3cfe into LemmyNet:main Jul 10, 2023
dessalines pushed a commit that referenced this pull request Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use canonical URLs
3 participants