Harden title filtering scope and migration notice safety#1
Harden title filtering scope and migration notice safety#1mehrazmorshed wants to merge 2 commits into
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 54 minutes and 35 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hide-titles.php`:
- Around line 126-127: The anchor elements using target="_blank" (e.g., the <a
class="link" href="https://profiles.wordpress.org/mehrazmorshed/"
target="_blank"> occurrences in hide-titles.php and the other similar anchors
around the given ranges) should include rel="noopener noreferrer" to prevent
reverse-tabnabbing; update every <a ... target="_blank"> instance (including the
ones mentioned at the other occurrences) to add rel="noopener noreferrer" while
preserving existing attributes and escaping via esc_url/esc_attr as currently
used.
- Around line 168-175: The legacy hide_titles() callback registered with
add_filter('the_title','hide_titles') unconditionally returns false when
get_option('hide-titles-option') == 'all' and thus blanks titles before the
guarded hide_titles_filter_title() can run; remove the entire legacy conditional
block that defines hide_titles() and its add_filter call, then incorporate the
global option check (get_option('hide-titles-option') === 'all') into
hide_titles_filter_title() so that the core filter logic only blanks titles
after the existing admin/feed/AJAX/JSON/main-query guards pass.
- Line 8: The plugin declares compatibility back to WP 4.4 but calls
wp_is_json_request() unguarded; wrap that call in a compatibility guard (e.g.,
if ( function_exists('wp_is_json_request') ? wp_is_json_request() : /* fallback
check */ ) or use function_exists('wp_is_json_request') to only call it when
available) or provide a small polyfill that emulates wp_is_json_request() for
older installs; update the code paths that reference wp_is_json_request() (the
call on Line ~293) to use this guarded call so older WP versions won't fatal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| * Plugin URI: https://wordpress.org/plugins/hide-titles/ | ||
| * Description: Remove Titles from Posts and Single Pages on WordPress. | ||
| * Version: 1.10 | ||
| * Requires at least: 4.4 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In WordPress core, which version introduced the function wp_is_json_request()?
💡 Result:
WordPress core version 5.0.0 introduced the function wp_is_json_request.
Citations:
- 1: https://developer.wordpress.org/reference/functions/wp_is_json_request/
- 2: https://typeerror.org/docs/wordpress/functions/wp_is_json_request
- 3: https://wp-kama.com/function/wp_is_json_request
🏁 Script executed:
fd -type f -name "hide-titles.php"Repository: mehrazmorshed/hide-titles
Length of output: 240
🏁 Script executed:
fd --type file -name "hide-titles.php"Repository: mehrazmorshed/hide-titles
Length of output: 300
🏁 Script executed:
fd hide-titles.phpRepository: mehrazmorshed/hide-titles
Length of output: 85
🏁 Script executed:
cat -n hide-titles.php | head -20Repository: mehrazmorshed/hide-titles
Length of output: 881
🏁 Script executed:
cat -n hide-titles.php | sed -n '290,300p'Repository: mehrazmorshed/hide-titles
Length of output: 352
🏁 Script executed:
rg "wp_is_json_request" hide-titles.phpRepository: mehrazmorshed/hide-titles
Length of output: 135
Guard wp_is_json_request() for stated minimum WordPress compatibility.
Line 8 declares support down to WordPress 4.4, but Line 293 calls wp_is_json_request() unconditionally, which was introduced in WordPress 5.0.0 and will cause a fatal error on older installs.
Suggested fix
- if ( is_feed() || wp_doing_ajax() || wp_is_json_request() ) {
+ $is_json_request = function_exists( 'wp_is_json_request' ) && wp_is_json_request();
+ if ( is_feed() || wp_doing_ajax() || $is_json_request ) {
return $title;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hide-titles.php` at line 8, The plugin declares compatibility back to WP 4.4
but calls wp_is_json_request() unguarded; wrap that call in a compatibility
guard (e.g., if ( function_exists('wp_is_json_request') ? wp_is_json_request() :
/* fallback check */ ) or use function_exists('wp_is_json_request') to only call
it when available) or provide a small polyfill that emulates
wp_is_json_request() for older installs; update the code paths that reference
wp_is_json_request() (the call on Line ~293) to use this guarded call so older
WP versions won't fatal.
| <a class="link" href="https://profiles.wordpress.org/mehrazmorshed/" target="_blank"> | ||
| <img class="center" src="<?php echo esc_url( plugin_dir_url( __FILE__ ) . 'img/author.png' ); ?>" width="128" height="128" alt=""> |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to all external target="_blank" links.
Several external links open in a new tab without rel, which leaves a reverse-tabnabbing gap in admin UI.
Suggested fix (pattern)
-<a class="link" href="https://profiles.wordpress.org/mehrazmorshed/" target="_blank">
+<a class="link" href="https://profiles.wordpress.org/mehrazmorshed/" target="_blank" rel="noopener noreferrer">Also applies to: 132-136, 141-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hide-titles.php` around lines 126 - 127, The anchor elements using
target="_blank" (e.g., the <a class="link"
href="https://profiles.wordpress.org/mehrazmorshed/" target="_blank">
occurrences in hide-titles.php and the other similar anchors around the given
ranges) should include rel="noopener noreferrer" to prevent reverse-tabnabbing;
update every <a ... target="_blank"> instance (including the ones mentioned at
the other occurrences) to add rel="noopener noreferrer" while preserving
existing attributes and escaping via esc_url/esc_attr as currently used.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several hardening measures and security improvements to the plugin, such as restricting title filtering to singular pages within the main query and adding permission checks for migration notices. The review feedback highlights opportunities to further improve the code by applying consistent hardening to the global title filter, enhancing theme compatibility for title detection, and optimizing the post-saving logic to handle revisions and use more idiomatic PHP patterns.
| function hide_titles() { | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
The hardening logic applied to hide_titles_filter_title (bailing for feeds, AJAX, admin, etc.) should also be applied to this global hide_titles filter. Currently, if the "Hide all titles" option is enabled, titles will be hidden site-wide, including in RSS feeds, AJAX responses, and the admin area, which contradicts the hardening goals of this PR. Additionally, the filter should accept and return the $title argument to follow WordPress standards.
function hide_titles( $title ) {
if ( is_admin() || is_feed() || wp_doing_ajax() || wp_is_json_request() ) {
return $title;
}
if ( ! in_the_loop() || ! is_main_query() || ! is_singular() ) {
return $title;
}
return '';
}| if ( ! in_the_loop() || ! is_main_query() ) { | ||
| return $title; | ||
| } | ||
|
|
||
| if ( ! is_singular() ) { | ||
| return $title; | ||
| } |
There was a problem hiding this comment.
Using in_the_loop() is a common way to target the main content, but it can be too restrictive for many themes. Some themes output the main title in a hero or header section before the loop starts (i.e., before the_post() is called), which would cause this filter to bail and fail to hide the title.
A more robust and compatible way to target the main title on singular pages is to compare the current post ID with the queried object ID while ensuring it is the main query.
if ( ! is_singular() || ! is_main_query() || (int) $id !== get_queried_object_id() ) {
return $title;
}| if ( 'post' !== get_post_type( $post_id ) && 'page' !== get_post_type( $post_id ) ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Calling get_post_type() multiple times is slightly inefficient. Storing the result in a variable is preferred. Additionally, it is best practice to bail early for post revisions to avoid unnecessary meta updates on revision objects during the save process.
if ( wp_is_post_revision( $post_id ) ) {
return;
}
$post_type = get_post_type( $post_id );
if ( 'post' !== $post_type && 'page' !== $post_type ) {
return;
}| return; | ||
| } | ||
|
|
||
| if ( array_key_exists( 'hide_titles_checkbox', $_POST ) ) { |
There was a problem hiding this comment.
In WordPress, checkboxes are only present in the $_POST array if they are checked. Using isset() is more idiomatic and slightly more efficient than array_key_exists() for this purpose.
if ( isset( $_POST['hide_titles_checkbox'] ) ) {1 participant
Motivation
Description
the_titlefilter inhide_titles_filter_titleto bail early for admin contexts, feeds/AJAX/JSON, non-main queries, out-of-loop calls, and non-singular views before reading the per-post_hide_titlemeta.hide_titles_save_metato only run for the supported post types by checkingget_post_type( $post_id )before updating or deleting_hide_title.ht_show_migration_noticeby gating it withcurrent_user_can( 'install_plugins' ), safely loadingis_plugin_active()viarequire_once ABSPATH . 'wp-admin/includes/plugin.php'when needed, and escaping the HTML message withwp_kses_post( __( ... ) ).Testing
php -l hide-titles.phpwas run and returned no syntax errors (passes).Codex Task
Summary by CodeRabbit
Release Notes