-
Notifications
You must be signed in to change notification settings - Fork 174
Add Media Resource Checker plugin #536
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
base: trunk
Are you sure you want to change the base?
Conversation
a189f1f to
d12ab9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is the right place for this file; maybe it should be in a higher directory to cover all plugin files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably define this into /.gitignore or just leave it in the project-specific area since it doesn't matter all that much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no strong opinion about where this file should be located 😄 Please feel free to move it elsewhere if necessary.
| * License: GPLv2 or later | ||
| */ | ||
|
|
||
| namespace WPOrg_Media_Resource_Checker; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use WordPressdotorg\Media_Resource_Checker instead?
|
|
||
| namespace WPOrg_Media_Resource_Checker; | ||
|
|
||
| use function WPOrg_Media_Resource_Checker\{ get_build_path, get_build_url }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the purpose of this is? Importing the functions defined in the current file into the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've imitated the implementation of the Learn plugin:
wordpress.org/wordpress.org/public_html/wp-content/plugins/wporg-learn/wporg-learn.php
Lines 25 to 41 in 8cafcd5
| /** | |
| * Shortcut to the build directory. | |
| * | |
| * @return string | |
| */ | |
| function get_build_path() { | |
| return PLUGIN_DIR . 'build/'; | |
| } | |
| /** | |
| * Shortcut to the build URL. | |
| * | |
| * @return string | |
| */ | |
| function get_build_url() { | |
| return PLUGIN_URL . 'build/'; | |
| } |
wordpress.org/wordpress.org/public_html/wp-content/plugins/wporg-learn/inc/events.php
Line 7 in 8cafcd5
| use function WPOrg_Learn\{ get_build_path, get_build_url }; |
We can also use an approach that doesn't use it if we don't need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9d72889
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably define this into /.gitignore or just leave it in the project-specific area since it doesn't matter all that much.
...public_html/wp-content/plugins/wporg-media-resource-checker/wporg-media-resource-checker.php
Outdated
Show resolved
Hide resolved
| // List of allowed domain regexes. | ||
| export const ALLOWED_DOMAINS = [ | ||
| { | ||
| authority: 'wordpress.org', | ||
| regex: /^(.*\.)?wordpress\.org$/, | ||
| }, | ||
| { | ||
| authority: 'wp.com', | ||
| regex: /^(.*\.)?wp\.com$/, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if wp.com should be allowed here, since WordPress.org != WordPress.com, and often using wpcom hosted images ends up invalid.
I can imagine this might also be intended for photon images? In which case, allowing ^(.*\.)?wp.com/(.*\.)?wordpress\.org/ could make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine this might also be intended for photon images?
Yes, that's right. For example, the handbook uses images that are actually delivered from the Photon CDN:
https://github.com/search?q=repo%3AWordPress%2Fdeveloper-plugins-handbook%20wp.com&type=code
In which case, allowing
^(.*\.)?wp.com/(.*\.)?wordpress\.org/could make sense?
I agree, fixed in e1db353.
Specific examples of allowed URLs can be found here.
| { | ||
| authority: 'wordpress.org', | ||
| regex: /^(.*\.)?wordpress\.org$/, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We likely should also allow the WordPress.org CDNs here:
| { | |
| authority: 'wordpress.org', | |
| regex: /^(.*\.)?wordpress\.org$/, | |
| }, | |
| { | |
| authority: 'wordpress.org', | |
| regex: /^(.*\.)?wordpress\.org$/, | |
| }, | |
| { | |
| authority: 'wordpress.org', | |
| regex: /^(.*\.)?w\.org$/, | |
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 302493e
Just to be sure, I have distinguished between the authority texts wordpress.org and w.org.
Closes: https://meta.trac.wordpress.org/ticket/8149
This PR adds a plugin that displays a warning for blocks that contain inappropriate external resources. This helps encourage contributors to upload resources when creating documentation by copying content from external documents, preventing media from potentially disappearing in the future.
How this plugin works
Additionally, the block toolbar will have a red icon button. Clicking that button will display a popover with details about the error.
Testing Instructions
Use the
.wp-env.override.jsonfile to map the plugin to your local wp-env environment:{ "plugins": [ "./your/plugins/plugin", "../path/to/wordpress.org/wordpress.org/public_html/wp-content/plugins/wporg-media-resource-checker" ] }Screenshot