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

Improve discussion page near canonical url #2397

Closed
w-4 opened this issue Oct 16, 2020 · 7 comments · Fixed by #2853
Closed

Improve discussion page near canonical url #2397

w-4 opened this issue Oct 16, 2020 · 7 comments · Fixed by #2853
Assignees
Milestone

Comments

@w-4
Copy link
Contributor

w-4 commented Oct 16, 2020

Bug Report

Current Behavior
In beta.13 and beta.14, there is a canonical url included in the hmtl document.
For example for https://discuss.flarum.org/d/187-word-association-game/1000
the canonical url is https://discuss.flarum.org/d/187-word-association-game?near=1000

As per #189 (comment) this causes search engines to crawl that URL.

Edit: the redirect works, my link was just broken.

@w-4 w-4 changed the title Remove discussion page near canonical url Improve discussion page near canonical url Oct 26, 2020
@w-4
Copy link
Contributor Author

w-4 commented Oct 26, 2020

We could do it the way the Discourse does. They also use a postnumber for the URL, but use a page parameter in the canonical url.
As an example:
For the first 20 posts the canonical would be https://discuss.flarum.org/d/187-word-association-game
for the next 20, it would be https://discuss.flarum.org/d/187-word-association-game?page=2

That is probably better for SEO, because of less links and no overlap.

@stale
Copy link

stale bot commented Jan 25, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Jan 25, 2021
@askvortsov1 askvortsov1 added this to the 0.1 milestone Mar 17, 2021
@stale stale bot removed the stale Issues that have had over 90 days of inactivity label Mar 17, 2021
@davwheat
Copy link
Member

I'm really confused by this. What's the issue here?

Using the /d/1233456/100 syntax could make search engines think it's a totally separate page from /d/123456 causing page duplication.

We should keep that canonical to prevent that, but we should also add rel=next to link to the page before that.

It might be work replacing this canonical with a ?page=x-based one, though, as the ?near=1000 seems to take me to post 950...

@enricoeur
Copy link

This is big mistake. Really i dont understand why was not fixed...

@askvortsov1
Copy link
Sponsor Member

I'm really confused by this. What's the issue here?

Using the /d/1233456/100 syntax could make search engines think it's a totally separate page from /d/123456 causing page duplication.

We should keep that canonical to prevent that, but we should also add rel=next to link to the page before that.

It might be work replacing this canonical with a ?page=x-based one, though, as the ?near=1000 seems to take me to post 950...

I like using page=x as the canonical URL. Post numbers can be messy.

I'll also note that the current no-js approach is missing next page / prev page buttons. THAT is quite problematic, and something that definitely needs to be fixed before stable.

@askvortsov1
Copy link
Sponsor Member

When redesigning, we should also make sure to fix #2130

@askvortsov1
Copy link
Sponsor Member

https://github.com/flarum/core/blob/f2168f939db15f58c1511b56b4d95c737b62ce1a/views/frontend/content/discussion.blade.php#L18-L24 is incorrect, because there won't be prev/next links on the API document for a single item. That's what's causing the missing pagination links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants