#34946 closed defect (bug) (fixed)
new comment redirects break anchors in Safari
Reported by: | chuckmoulton | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.4.1 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Comments | Keywords: | fixed-major |
Focuses: | Cc: |
Description
WordPress has the following setting:
[x] Break comments into pages with [ 50 ] top level comments per page and the [last] page displayed by default
In previous versions of WordPress, changing that setting resulted in recent comment links appended with "/comment-page-1/" between the end of the post URL and the # anchor tag to the specific comment. This triggers a server redirect to the post URL, stripping "/comment-page-1/" off. Most browsers handle it fine... Safari across all platforms chokes. Safari redirects to the post without the anchor tag, meaning the user is at the top of the page rather than scrolled down to the particular comment.
Original recent comments link:
example.com/2015/12/example-post/comment-page-1/#comment-1234567
Redirects (Firefox, Chrome, IE, every sensible browser):
example.com/2015/12/example-post/#comment-1234567
Redirects (Safari):
example.com/2015/12/example-post/
The logical solution was to uncheck the setting:
[_] Break comments into pages with [ 50 ] top level comments per page and the [last] page displayed by default
Unfortunately, the WordPress 4.4 update changed this. Now the recent comments redirect happens whether or not that setting is checked. This breaks recent comments anchor tags for Safari. Sites with many comments now leave iPhone users forced to scroll to the bottom of the post in order to find the linked to comment.
Obviously, the real problem is Safari, which has been broken for a decade. But the reality is mobile users are a growing part of the WordPress audience... sites can't jettison their iPhone users just because Safari has a bug. Safari isn't likely to fix the bug on their end anytime soon.
Please provide an option for disabling the "/comment-page-1/" redirects.
Attachments (1)
Change History (14)
This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.
9 years ago
#4
@
9 years ago
@boonebgorges
The recent comments widget puts a sidebar on a WordPress blog with the 20 most recent comments. Clicking on any of those 20 comments results in a different link depending on the configuration.
Configurations:
1) If pre- WordPress 4.4 and "break comments into pages" is unchecked, then this link:
example.com/2015/12/example-post/#comment-1234567
2) If pre- WordPress 4.4 and "break comments into pages" is checked, then this link:
example.com/2015/12/example-post/comment-page-1/#comment-1234567
3) If WordPress 4.4 -- regardless of the "break comments into pages" setting --, then this link:
example.com/2015/12/example-post/comment-page-1/#comment-1234567
When "comment-page-1" is in the link, the WordPress server redirects. Safari drops anchor tags from any server redirect.
Redirect behavior:
A) If (1), then no redirect.
B) If [ (2) or (3) ] and most browsers (Chrome, Firefox, IE, etc.), then this redirect:
example.com/2015/12/example-post/#comment-1234567
C) If [ (2) or (3) ] and Safari, then this redirect:
example.com/2015/12/example-post/
I am trying to avoid (C) because many readers and active participants of the blog use iPhones. Before WordPress 4.4 came out, I could avoid (C) by choosing configuration (1) instead of (2). After the WordPress 4.4 update, I am forced to use configuration (3), which means I get behavior (C).
#5
@
9 years ago
- Keywords needs-testing added
- Milestone changed from Awaiting Review to 4.4.1
Thanks, @chuckmoulton. These details are very helpful.
There are two things going on here. First, there is a regression in 4.4 that causes get_comment_link()
to use the 'cpage=1' query var (or 'comment-page-1', when pretty permalinks are enabled) when comment pagination is disabled. This should absolutely be fixed. Could you please test 34946.diff to be sure that it's resolving the problem as you see it?
The other problem is a broader one. If the canonical comment-page-1 resolution is causing problems, then we should try to avoid using comment-page-1 in permalinks when possible, even when comment pagination is enabled. When the newest comments are shown first, we cannot eliminate comment-page-1; it's a critical piece of the URL. But when *oldest* comments are shown on the first page, it should be possible to eliminate comment-page-1. I tried doing this in 4.4, but ran into some related issues. Maybe it's something to try again in 4.5.
#6
@
9 years ago
@boonebgorges
I'm sorry, I am not currently in a position to test the change. I don't have a testing server, I'm not the lone decision maker in upgrades/downgrades, and my site has chosen to downgrade to 4.3 for this as well as several other bugs. I reported this particular bug because it affected me the most and I encouraged others to report the other bugs*, but I don't know if they did. Until all the bugs seem acknowledged and resolved, we will be sticking with 4.3.
However, your description above seems quite sensible and based on looking at the code you linked (without running it) I think you're right. If anyone tries your code on a server and links it here, I'd be happy to test it as a user with an iPhone / Safari.
Sorry I can't be of more help. I greatly appreciate your looking into this.
- Note: The other bugs concern out of order / non-chronological comments from comment replies in the admin console when threaded (nested) comments are not enabled.
#7
@
9 years ago
@boonebgorges double checking per comment:5 request, 34946.diff is working for me against trunk.
- comment pagination enabled: always includes
cpage
orcomment-page-#
in link - comment pagination disabled: never includes
cpage
orcomment-page-#
in link
Tested from the latest comments widgets.
@chuckmoulton which version of Safari are you using? I'm unable to replicate the redirect error in Safari 9.0.2/OSX 10.11.2.
#8
@
9 years ago
- Keywords needs-testing removed
@peterwilsoncc Thanks so much for testing. It's interesting that you can't reproduce this issue in recent versions of Safari. But the behavior is a regression in the behavior of get_comment_link()
in 4.4, regardless of whether the Safari bug exists.
#9
@
9 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 35933:
#10
@
9 years ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for 4.4.1.
#13
@
9 years ago
(Using 4.4 trunk, no comment pagination)
I still have to check, but it looks like after submitting a comment, WordPress redirects to .../comment-page-1/#comment-123
, which redirects to .../#comment-123. Not a big deal to me, but Safari strips off
#comment-123` in the second redirect, so the user isn't taken directly to their comment.
comment-page-1
is not part of the comment links anywhere else, so I'm not sure where this comes from.
EDIT: Looks like my files were outdated, oops.
@chuckmoulton Welcome to Trac! Thanks for the detailed bug report.
It's not good that 4.4 resulted in a regression for your specific use case. However, it sounds to me like your use case was a total hack - you shouldn't have to turn off comment pagination just to placate Safari/WP.
I'd like to take a step back and try to solve this in a more sane way. If the problem has to do with *redirection*, then let's try to eliminate the problematic redirections. Can you please give more detail on the user flow that results in the page redirect? What link are you clicking - the permalink as sent in an email notification, in a recent comments widget, or somewhere else? What is the URL of the link? And what is the page that it redirects to? You have sketched it above, but I'm still not 100% clear on the difference between 4.3 and 4.4 in this regard.