Skip to content

Plugin Directory: Treat the plugin file header like the readme section it replaces on import - #881

Open
mcliwanow wants to merge 5 commits into
WordPress:trunkfrom
mcliwanow:plugin-import-header-fallbacks
Open

Plugin Directory: Treat the plugin file header like the readme section it replaces on import#881
mcliwanow wants to merge 5 commits into
WordPress:trunkfrom
mcliwanow:plugin-import-header-fallbacks

Conversation

@mcliwanow

@mcliwanow mcliwanow commented Sep 7, 2026

Copy link
Copy Markdown

Why

When a plugin has no readme, the importer builds the description section from the plugin file's Description: header and stores the header as written. Every other value that reaches that column is narrowed first: readme sections go through the parser's section allow-list in filter_text(), and the Name: header three lines up in the same block goes through strip_tags(). The header fallback was the one exception, in a block that otherwise treats every input the same way.

What changed

  • The description fallback in cli/class-import.php now runs through Readme\Parser::filter_text(), so a header stands in for a readme section under the same rules as a section. filter_text() goes from protected to public for that. The alternative was a second copy of the allow-list in the importer, which would drift.
  • A header that filters down to nothing no longer writes an empty description section over the stored one. The emptiness check used to look at the raw header while the section was built from the filtered text, so a header made only of markup outside the list stored a bare section marker. The existing description now stays, as it does when the header is absent.
  • A parser test pins what filter_text() keeps (the section tags, a with href/title/rel) and drops (other attributes, other elements, HTML comments), and that it balances an unclosed tag.
  • Visible effect: a plugin with no readme whose header carries markup outside the section list (<div>, <img>, class=, style=) shows that markup as text on its page. The excerpt, the header_description meta and the upload form are unchanged.

Testing

wp-env environments/plugin-directory (WP trunk), the real Import::import_from_svn() run against three local plugin directories with only the SVN export step replaced, on trunk and on this branch, container restarted between. The same three posts each time, seeded as approved; every import moved them to publish.

plugin directory trunk branch
no readme, header with a <div>, a link, and id and other attributes on both post_content holds the header as written; the page renders the <div> with every attribute post_content keeps the link with its href only, the rest is text; the page renders one paragraph with a plain link
no readme, header with <strong>, <a href title>, <code>, a bare & and a bare < stored as written, the page carries a raw < the three elements kept, & and < encoded
header plus a readme readme section stored, header ignored identical

The excerpt column is not touched by this change and stored the same value on both.

PHPUnit on the branch, after merging trunk: 435 tests, 1476 assertions, passing (Test_Readme_Parser 18 / 29, five of them new). Changed lines lint clean with phpcs.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved plugin description imports when readme sections are unavailable.
    • Preserved existing description content when filtering would otherwise produce an empty result.
    • Strengthened readme text cleaning by removing HTML comments and unsafe markup while retaining permitted formatting.
    • Improved handling of incomplete HTML tags to produce balanced, consistent content.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 522fc86b-0f52-41f1-9a0b-a3b0f597a551

📥 Commits

Reviewing files that changed from the base of the PR and between 6048bc2 and 63c19a1.

📒 Files selected for processing (3)
  • wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-import.php
  • wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php
  • wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

Parser::filter_text() is now public and removes HTML comments after sanitization. Fallback descriptions from plugin headers use this filter and do not replace existing content when filtering produces an empty result. Tests cover the filtering behavior.

Changes

Readme filtering

Layer / File(s) Summary
Parser filtering contract and tests
wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php, wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php
Parser::filter_text() is public and removes HTML comments after wp_kses() filtering. Data-provider tests verify markup preservation, attribute and element removal, tag balancing, and comment removal.
Fallback description filtering
wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-import.php
Fallback descriptions from plugin headers pass through filter_text(). The importer preserves existing content when the filtered description is empty.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 63c19

Plugin header fallback descriptions now use the existing readme sanitization behavior without replacing stored content when sanitization produces no content. No current merge-blocking risk remains.

Suggested reviewers: obenland

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: treating the plugin file header as the replacement readme section during import.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…n it replaces on import

When a plugin has no readme, the importer fills the description section
from the plugin file's `Description:` header. The header went in as
written, while a readme section is narrowed by the readme parser before
it is stored, and the `Name:` header on the neighbouring line already
goes through `strip_tags()`.

The description fallback now runs through `Readme\Parser::filter_text()`,
which becomes public for that instead of the importer carrying a copy of
the section allow-list.

A parser test pins what `filter_text()` keeps and drops.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mcliwanow
mcliwanow force-pushed the plugin-import-header-fallbacks branch from 2763803 to 0937ad6 Compare September 8, 2026 07:16
@mcliwanow mcliwanow changed the title Plugin Directory: Treat the plugin file header like the readme value it replaces on import Plugin Directory: Treat the plugin file header like the readme section it replaces on import Sep 8, 2026
@mcliwanow mcliwanow self-assigned this Sep 8, 2026
@mcliwanow
mcliwanow marked this pull request as ready for review September 8, 2026 08:45
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props mcliwanow.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@mcliwanow
mcliwanow requested review from KokkieH and bor0 September 9, 2026 11:39
mcliwanow and others added 3 commits September 9, 2026 17:24
…o nothing

The readme-less fallback tested the raw header for emptiness but built the
section from the filtered value, so a header made only of markup outside
the readme list wrote a section marker with nothing after it and replaced
whatever the row held. The marker is now only written when the filtered
text is non-empty; otherwise the existing description stays, as it does
when the header is absent.

The parser test also pins that `filter_text()` drops HTML comments, which
it does since the screenshot caption change on trunk.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant