#25268 closed defect (bug) (fixed)
Re-enable processing of scheduled and private posts in _fix_attachment_links()
Reported by: | jond3r | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.7 | Priority: | normal |
Severity: | normal | Version: | 3.3.2 |
Component: | Media | Keywords: | has-patch needs-unit-tests |
Focuses: | Cc: |
Description
The purpose of _fix_attachment_links()
is to change attachment links in post content to its final "pretty" format, if non-default permalinks are enabled. The function is called late in the post update cycle, and for posts transitioned from draft to published, the attachment links are fixed by calling get_attachment_link()
with the post in a 'publish' status. While a post is in draft status, get_attachment_link()
will return permalinks of the form http://mysite.com/?attachment_id=xxx, and when the post is published, get_attachment_link()
will typically return something like http://mysite.com/parent-post-slug/attachment-slug/, i.e. in a pretty format.
At present however, _fix_attachment_links()
will return prematurely, without accomplishing its task, for scheduled post and private posts. This is caused by the "sanity check" early up in the function which returns if pretty permalinks are not enabled or the post_status is not 'publish'. This sanity check was introduced in [20308] (v. 3.3.2), ticket #13429 together with a major revamp of the function.
The proposed attached patch modifies the sanity check to ignore posts of type 'attachment', and with a status of 'draft', 'pending' or 'auto-draft'. This latter check is the same as done in get_permalink()
to determine the type of link to return (pretty or not).
There are some other post statuses that one might suspect being affected by this patch. Attachments have a status of 'inherit', and are explicitly excluded in the sanity check. Revisions also have a status of 'inherit', but will never end up in _fix_attachment_links()
. The same applies for trashed posts, with a status of 'trash'. Thus only posts with a status of 'publish', 'private' or 'future' are let through the sanity check.
Example procedure how to reproduce for scheduled posts:
- Ensure that non-default permalinks are chosen in the Permalink Settings
- Create a new post, and add a title.
- Add an unattached image to the content, and ensure that the Link To drop-down shows "Attachment Page". (The image is unattached if the URL in the text field below the drop-down contains "/?attachment_id=").
- Schedule the post some minute in the future by clicking "Edit" next to "Publish immediately". Press OK.
- Press "Schedule".
- Observe that the attachment link URL is not changed in the content area (it still contains a "?"). The same applies when viewing the post after (and before) the post is transitioned to published.
Contrast this to the behavior when the post is published immediately. Then the attachment link is changed (prettified) when "Publish" is pressed.
Though not explicitly tested, I'm quite sure this is a regression introduced in [20308].
The function _fix_attachment_links()
dates back to [3145] (v. 2.0.0), ticket #1870, under the name fix_attachment_links()
in the now deprecated file admin-functions.php. In [5542] (v. 2.2.1) it was moved to /includes/post.php, and the function name was prepended by an _.
The patch also changes the @since tag to 2.0.0 to reflect this, and fills in the missing type info for @param and @return. Possibly the @since tag should be 2.2.1?
The material for this this ticket emerged in the process of refreshing #22679.
Attachments (2)
Change History (8)
#4
@
11 years ago
The reason for blacklisting draft-like statuses in the original patch was to mimic the check done in get_permalink()
, which ultimately determines whether the attachment permalink is prettified. However, the whitelisting approach works equally well as long as there are no new publish-like statuses introduced.
I'll try to come up with a few unit tests, but don't expect anything very soon.
25268.diff looks good. It will be really really helpful to have few testcases for this.
In short:
_fix_attachment_links()
has to run after the post's permalink is finalized. This happens when the post is either published or scheduled or set as private. Thinking it would be better to "whitelist" thepublish
or equivalent post_status rather than "blacklist"draft
,auto-draft
andpending
.