Opened 9 years ago
Closed 9 years ago
#34455 closed defect (bug) (fixed)
Additional Filters for Press This
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.5 | Priority: | high |
Severity: | normal | Version: | 4.2 |
Component: | Press This | Keywords: | has-patch |
Focuses: | Cc: |
Description
I've added 3 filters for the Press This class:
- 'press_this_save_post_content'
Applied right after the side_load_images in order to allow potential side loads of other types of media.
Example use case: side load non-image media, such as audio or video.
- 'press_this_useful_html_elements'
Allows filtering of currently hardcoded array of HTML elements allowed in fetch_source_html step for special cases where additional HTML elements need to be kept.
Example use case: HTML5 elements, such as amp-img, that someone wants to pull in.
- 'press_this_suggested_content'
A filter for the content right before it's passed to the editor and presented to the user.
Example use case is when someone stored posts in a different, non-HTML format, such as Markdown, this is essential.
Attachments (3)
Change History (29)
#2
follow-up:
↓ 4
@
9 years ago
- Keywords 2nd-opinion added
- Version changed from 4.3.1 to 4.2
Howdy Cadeyrn! My apologies for not replying to this earlier and thank you for the patch.
As a best practice, it is best to create a patch from the checkout root so the full path is there. The filters do sound useful.
#3
@
9 years ago
- Milestone changed from Awaiting Review to 4.5
- Owner set to kraftbj
- Status changed from new to accepted
#4
in reply to:
↑ 2
@
9 years ago
Thank you for the cleanup and the help; next time I'll include the full path, thank you.
This ticket was mentioned in Slack in #core by jorbin. View the logs.
9 years ago
#8
@
9 years ago
- Milestone changed from 4.5 to Future Release
No real activity in multiple weeks, so this is going to miss 4.5
#9
@
9 years ago
@jorbin What is lacking? The patch is good, as far as I know, and it was marked commit
.
#10
@
9 years ago
- Milestone changed from Future Release to 4.5
@kraftbj it was the lack of activity and the fact that it's been 7 weeks with the commit tag and no commit.
It is lacking the version in the since, but that is minor. You showed interest and it's before the deadline, so I'll finish up the docs and commit it.
#12
@
9 years ago
@jorbin I think we should probably rename the press_this_useful_html_elements
hook to press_this_allowed_tags
. It is, in and of itself a list of allowed tags. I don't know that they're so much "useful" as "allowed".
#13
follow-up:
↓ 14
@
9 years ago
I'm not sure that any of these filters is particularly useful.
press_this_save_post_content
is yet another filter that runs when a post is saved. There are already a few other filters that can be used to do exactly the same.press_this_useful_html_elements
runs only when the content _is not_ passed by the bookmarklet, i.e. most of the time it cannot be used. As far as I see it can be useful only when repurposing Press This for other user cases. However that is not supported, and not sure if we should encourage it.press_this_suggested_content
is very similar topress_this_suggested_html
(even the name sounds confusing when comparing them). It filters the return ofpress_this_suggested_html
after it has been wrapped in HTML tags. Again, only useful if repurposing Press This to do something that was not intended.
If plugins want to repurpose Press This, they are free to do so. However this is not the intended use and will probably break when Press This is updated.
#14
in reply to:
↑ 13
;
follow-up:
↓ 15
@
9 years ago
@azaozz
I'm not sure that any of these filters is particularly useful.
press_this_save_post_content
is yet another filter that runs when a post is saved. There are already a few other filters that can be used to do exactly the same.
Is sideloading from the referenced url (which lead to Press This) possible on regular post save?
press_this_useful_html_elements
runs only when the content _is not_ passed by the bookmarklet, i.e. most of the time it cannot be used. As far as I see it can be useful only when repurposing Press This for other user cases. However that is not supported, and not sure if we should encourage it.
Why would it be repurposing just because some might find other HTML elements useful as well?
press_this_suggested_content
is very similar topress_this_suggested_html
(even the name sounds confusing when comparing them). It filters the return ofpress_this_suggested_html
after it has been wrapped in HTML tags. Again, only useful if repurposing Press This to do something that was not intended.
This is not true. I'm using Markdown when writing posts and without this, I can't do so in Press this.
#15
in reply to:
↑ 14
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
@cadeyrn
Is sideloading from the referenced url (which lead to Press This) possible on regular post save?
I may be missing something but "sideloading" is possible at any time, when saving a post or not. You have a choice of media_sideload_image()
, media_handle_sideload()
or wp_handle_sideload()
depending on how much control you want to have.
Why would it be repurposing just because some might find other HTML elements useful as well?
What I mean is that the press_this_useful_html_elements
filter only runs when not using the bookmarklet and an URL is typed in PT directly. This is a compatibility mode that's rarely used. So this filter will never run in normal, intended use of PT. In addition it is somewhat confusing as plugin authors may expect it to always run.
This is not true. I'm using Markdown when writing posts and without this, I can't do so in Press this.
The press_this_suggested_content
filter has nothing to do with using Markdown (or anything else) in the editor. If you don't want the default HTML tags, you can remove them completely or replace with whatever you need by using press_this_suggested_html
. I don't see why we need another filter for almost the same thing. In addition press_this_suggested_content
is harder to use as the only way would be to parse the HTML with regexp, and that is... not good :)
Reopening for reconsideration. At least the names need to be better.
#18
follow-ups:
↓ 19
↓ 20
@
9 years ago
- Keywords has-patch added; needs-patch removed
- Priority changed from normal to high
- Type changed from enhancement to defect (bug)
In 34455.1.patch:
- Rename
press_this_save_post_content
topress_this_save_post
and pass all post data to it. This is still marginally useful as there are at least two other "save post" filters that can be used, however if we must have it at least lets filter all of the post data.
- Remove
press_this_useful_html_elements
. It's misleading as it runs only in compatibility mode. Also we are looking at keeping some of the HTML elements in the bookmarklet, then we will have to sync this or remove all, depending on the direction that goes.
- Remove
press_this_suggested_content
. It is completely useless as all of the suggested content can be controlled better by usingpress_this_suggested_html
.
#19
in reply to:
↑ 18
@
9 years ago
Replying to azaozz:
- Remove
press_this_suggested_content
. It is completely useless as all of the suggested content can be controlled better by usingpress_this_suggested_html
.
No, it can't. press_this_suggested_html
runs before the content is extended with additional strings.
https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-admin/includes/class-wp-press-this.php#L1171
#20
in reply to:
↑ 18
;
follow-up:
↓ 21
@
9 years ago
Replying to azaozz:
- Remove
press_this_suggested_content
. It is completely useless as all of the suggested content can be controlled better by usingpress_this_suggested_html
.
I can't find any filter for the $text
variable in get_suggested_content
. It is not part of the $default_html
that is filtered with press_this_suggested_html
.
If the press_this_suggested_content
is getting dropped, could that $text be included in the filtered array?
#21
in reply to:
↑ 20
;
follow-up:
↓ 23
@
9 years ago
Replying to cadeyrn:
I can't find any filter for the
$text
variable inget_suggested_content
.
The press_this_suggested_html
filter lets you modify (or remove) the HTML tags. Then sprintf()
is used to add the content at the specified places in the modified strings. I'm not sure how exactly you're trying to use this, but to get only the $text
variable in the content you can do:
add_filter( 'press_this_suggested_html', 'my_press_this_content' ); function my_press_this_content( $array ) { return array( 'quote' => '%s' ); }
#22
@
9 years ago
Thumbs up to 34455.1. @azaozz's modifications makes sense to me and simplifies the available filters.
#23
in reply to:
↑ 21
;
follow-up:
↓ 24
@
9 years ago
Replying to azaozz:
OK, I got it.
The end result can be filtered with press_this_suggested_html
; however, the actual content, that is sprint-ed into the 'quote' array element ( $text
variable within get_suggested_content
) can be filtered with the press_this_data
filter, called in merge_or_fetch_data
, so there is no need for a final filter.
Next time I'll spend more time learning the code and adding documentation instead of introducing unneeded filters; thank you for your time, @azaozz.
#24
in reply to:
↑ 23
@
9 years ago
Replying to cadeyrn:
Next time I'll spend more time learning the code and adding documentation instead of introducing unneeded filters; thank you for your time.
Don't beat yourself up—I didn't catch there was existing ways to do what you wanted either. Your ticket still added a filter to make Press This more extendable. Thank *you* for your time and for sticking with this issue.
#25
@
9 years ago
Yeah, it is partially my fault too. Should have added more detailed explanation in the inline docs.
The press_this_suggested_html
filter is not the most straightforward way of controlling the suggested content for the editor but gives more granular access that is quite better than parsing the HTML with regex.
patch for class-wp-press-this.php for additional filters