Make WordPress Core

Opened 4 years ago

Last modified 40 hours ago

#53076 accepted enhancement

Press This: Add filters to allow custom Press This plugins

Reported by: kraftbj's profile kraftbj Owned by: kraftbj's profile kraftbj
Milestone: 6.9 Priority: normal
Severity: normal Version: 4.9
Component: Editor Keywords: has-test-info has-patch needs-dev-note
Focuses: Cc:

Description

The Press This feature of WordPress was spun out of Core in 4.9 as into a "canonical plugin". The Core side code presumes the canonical plugin is the only implementation.

If a site owner wants a custom version of Press This, they are required to hack the plugin in some way (set the version super high and edit the files, replace it with something of a high version, etc to ensure the plugin is updated back to stock by WordPress).

This also limits what site owners can do—selective versions of Press This based on who is logged in, A/B tests of different Press This's, etc.

Request a few filters added to wp-admin/press-this.php to allow for alternative implementations.

Change History (19)

This ticket was mentioned in PR #1207 on WordPress/wordpress-develop by kraftbj.


4 years ago
#1

  • Keywords has-patch added

Adds filters to wp-admin/press-this.php to allow for custom Press This plugins to be used without special hacks.

Trac ticket: https://core.trac.wordpress.org/ticket/53076

This ticket was mentioned in Slack in #core by chaion07. View the logs.


4 years ago

#3 @poena
4 years ago

  • Keywords needs-testing needs-testing-info added

Hi
This ticket was brought up during a bug scrub, but we were not sure how to continue to test this without first creating a custom press-this plugin to test the filter with.

Do you have a sample plugin code for this purpose? Or other testing instructions?

#4 @poena
4 years ago

Hi @kraftbj will you be able to provide further testing instructions before the 5.8 feature freeze?

#5 @kraftbj
4 years ago

  • Milestone changed from 5.8 to Future Release
  • Owner set to kraftbj
  • Status changed from new to accepted

Hi @poena. I'll move it out of the milestone; a bit of a chicken and the egg where I was looking at add the filters before having the plugin to utilize them, but makes sense to wait.

#6 @SirLouen
2 months ago

  • Keywords has-testing-info added; needs-testing needs-testing-info removed

Combined Test Report and Patch Testing

Description

🟠 This report validates that the patch is working with a minor error

I have simply replicated the original "Press This" plugin and added a very trivial modification. We have to remember that the idea here is simply adding the capacity to a user to modifying the "Press This" plugin without risking an update, hence simply changing the basic namespace elements of the plugin. This patch adds the possibility of modifying all the static strings for this purpose.

Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/1207.diff

Environment

  • WordPress: 6.8-beta1-59933-src
  • PHP: 8.2.25
  • Server: nginx/1.27.2
  • Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.25)
  • Browser: Chrome 133.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty 2.8
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0


Steps to Reproduce the Patch

  1. First install a slightly modified version of Press This. In this case I'm using this one I've uploaded to my repo for the testing purpose: https://github.com/SirLouen/press-this-testing/archive/refs/tags/1.0.0.zip
  2. Install the Bookmarklet and use the plugin
  3. 🐞 Bug occurs as expected https://i.imgur.com/lRvokvf.png


Actual Results After adding the patch

First filter: press_this_plugin_slug

  1. For the first filter we install the first modified version of this Press This plugin: https://github.com/SirLouen/press-this-testing/archive/refs/tags/1.0.1.zip
  2. ❌ The test fails with an error.

The problem is that the code forces the use of not only the first filter, but also the second filter. I have added a new patch to sort this out, so the user can simply use the first filter to set a new slug (which is, in fact, the main purpose of this feature, adding the possibility of having an alt-version of Press This without having to edit the versioning over the official Press This plugin)

Second filter: press_this_plugin_slug

  1. We install the modified version of this Press This plugin with this test: https://github.com/SirLouen/press-this-testing/archive/refs/tags/1.0.2.zip
  2. ✅ The test passes correctly

Third filter: press_this_execution_func

  1. We install the modified version of this Press This plugin with this test: https://github.com/SirLouen/press-this-testing/archive/refs/tags/1.0.3.zip
  2. ✅First we do a function for the filter without returning a callable and returns the appropriate error as expected: https://i.imgur.com/ksOUYii.png
  3. ✅ Second we do a function returning a new callable with a slight class naming mod and the test passes correctly


Supplemental Artifacts

All the required artifacts have been linked in the corresponding tests.

#7 @SirLouen
2 months ago

  • Keywords changes-requested added

Added the changes to the Github PR as a comment (I don't have push perms)

Last edited 2 months ago by SirLouen (previous) (diff)

#8 @SirLouen
2 months ago

  • Keywords dev-feedback added

#9 follow-up: @SirLouen
2 months ago

Raising attention of @azaozz since he was the creator of press-this.php and current Editor component maintainer.

#10 in reply to: ↑ 9 ; follow-up: @azaozz
8 weeks ago

  • Keywords dev-feedback removed

Replying to SirLouen:

Hi @SirLouen thanks for the ping here. Frankly I'm not so sure if it is a good idea to still use bookmarklets (i.e. elevated JS code in a browser bookmark). They were pretty popular 10-15 years ago, but have some security problems and generally the way browsers work has changed a lot since then.

Nevertheless, looking at the PR maybe it can be much simpler? Seems all it needs is to allow plugins to replace the wp_load_press_this() function? That can easily be done by running that function on an action, for example on plugins_loaded. Then any plugin will be able to remove the default core callback and add its own.

So generally the code to achieve this would be to replace the loose wp_load_press_this() with something like:

add_action( 'plugins_loaded', 'wp_load_press_this' );

(This idea is untested. Seems to be working at first look but there may be something that is expected to run before plugins_loaded.)

Last edited 8 weeks ago by azaozz (previous) (diff)

#11 in reply to: ↑ 10 @SirLouen
8 weeks ago

Replying to azaozz:

Replying to SirLouen:

Hi @SirLouen thanks for the ping here. Frankly I'm not so sure if it is a good idea to still use bookmarklets (i.e. elevated JS code in a browser bookmark). They were pretty popular 10-15 years ago, but have some security problems and generally the way browsers work has changed a lot since then.

Nevertheless, looking at the PR maybe it can be much simpler? Seems all it needs is to allow plugins to replace the wp_load_press_this() function? That can easily be done by running that function on an action, for example on plugins_loaded. Then any plugin will be able to remove the default core callback and add its own.

So generally the code to achieve this would be to replace the loose wp_load_press_this() with something like:

add_action( 'plugins_loaded', 'wp_load_press_this' );

(This idea is untested. Seems to be working at first look but there may be something that is expected to run before plugins_loaded.)

The reality is that the plugin is almost abandoned but had a minor version update 4 months ago.
https://github.com/WordPress/press-this/issues and one issue some months ago

Not the most sexy plugin in 2025, not gonna lie :)

This said, I think that the original idea of @kraftbj was simply a copy-paste of the initilization code as a filter. A simply posted a PR with the adapted original code from him

If anyone wants to provide a new PR, I could try to test it. This action/filter hook-based reports are pretty weird to test though (hence most of the stalled reports with patches are mostly of this type)

#12 @SirLouen
8 weeks ago

  • Keywords needs-refresh added; has-patch removed

#13 @kraftbj
7 weeks ago

  • Keywords has-patch added; changes-requested needs-refresh removed

The patch has been brought up to current trunk and included a suggestion from @SirLouen.

Also, for the sake of saying, I'm happy to review PRs into the canonical Press This plugin too! You might need to ping me to nudge me to look. :)

I'm also happy to spin up a separate PR for the hooked approach that @azaozz suggested. I'll work on that now.

Last edited 7 weeks ago by kraftbj (previous) (diff)

This ticket was mentioned in PR #8612 on WordPress/wordpress-develop by @kraftbj.


7 weeks ago
#14

This is an alternative approach of the solution in #1207 after feedback from @azaozz. This would replace the freefloating execution of the load function behind a new press_this_init action.

In admin-filters, add a default action which plugins can unhook, etc.

#15 follow-up: @kraftbj
7 weeks ago

  • Milestone changed from Future Release to 6.9

I agree that this alternative is cleaner. @SirLouen, what do you think?

#16 in reply to: ↑ 15 @SirLouen
6 weeks ago

  • Keywords needs-testing added

Replying to kraftbj:

I agree that this alternative is cleaner. @SirLouen, what do you think?

Let me test this with my testing plugin and I will get back asap.

#17 @SirLouen
6 weeks ago

  • Keywords needs-dev-note added; needs-testing removed

✅ I've tested with my plugin and everything is working well as expected.

I confirmed the first patch, mainly because the initial use-case, was to have quick solution to add some tweaks to the Press-This OG plugin.

With this new patch, from a code perspective as @azaozz suggested, it's much cleaner, but it's a hassle for the new plugin creator (at minimum, needing to copy/paste the wp_load_press_this, to initialize the same the precursor instance if he is only adding some tweaks to the OG).

Even with just one filter of the starting 3 (the slug one), you could perfectly achieve the initial use-case as long as you preserved the same loading filename and same class name in your tweaked plugin.

Let's remember which was the initial use-case:

If a site owner wants a custom version of Press This, they are required to hack the plugin in some way (set the version super high and edit the files, replace it with something of a high version, etc to ensure the plugin is updated back to stock by WordPress).

With a different slug, this is perfectly sorted. And in terms of total code, is even less than the action hook solution. And I seriously doubt that anyone in 2025 is going to invest time on revamping a full Press-This plugin, instead of just, either PRing the OG or just adding some little tweaks.

Furthermore, with this new version with a hook action, I believe that a dev-note is required 100% to explain what's going on. With filter hooks, I feel that a dev note is optional, but it would be nice as well to inform about this new "feature".

Last edited 6 weeks ago by SirLouen (previous) (diff)

@kraftbj commented on PR #1207:


4 weeks ago
#18

Closing this to defer to #8612

#19 @wordpressdotorg
40 hours ago

  • Keywords has-test-info added; has-testing-info removed
Note: See TracTickets for help on using tickets.