Make WordPress Core

Opened 2 years ago

Closed 22 months ago

#56581 closed defect (bug) (reported-upstream)

Wrong doc in WP_Block_Parser class

Reported by: chouby's profile Chouby Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Priority: normal
Severity: trivial Version: 5.0
Component: Editor Keywords: has-patch
Focuses: docs Cc:

Description

WP_Block_Parser::parse() is documented as returning WP_Block_Parser_Block[] but it returns an array or arrays.

WP_Block_Parser::$output is also documented as being WP_Block_Parser_Block[] but is also an array of arrays.

This is visible in several places of the parse() method where $this->output is explicitely casted to an array.

Additionnally, I noticed that several @since seem to refer to Gütenberg versions instead of WordPress versions (as version numbers are < 5.0.0).

Attachments (3)

56581.patch (651 bytes) - added by Chouby 2 years ago.
56581-since.patch (1.5 KB) - added by Chouby 2 years ago.
56581-since-2.patch (1.6 KB) - added by Chouby 2 years ago.

Download all attachments as: .zip

Change History (23)

@Chouby
2 years ago

@Chouby
2 years ago

#1 @Chouby
2 years ago

  • Keywords has-patch added

I attached two patches.

  • The first one changes WP_Block_Parser_Block[] to array[].
  • The second one fixes @since. I separated from the first one as it was not the primary goal of this ticket and looses some changelog information which seems related to early Gütenberg development.

#2 follow-up: @Chouby
2 years ago

I wrongly assumed that everything was introduced in 5.0.0. The second patch for @since fixes that as some chnages were introduced in 5.1.0.

#3 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.1

#4 @SergeyBiryukov
2 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#5 in reply to: ↑ 2 @SergeyBiryukov
2 years ago

Hi there, thanks for the patches!

Replying to Chouby:

I wrongly assumed that everything was introduced in 5.0.0.

Upon a closer look, these properties and methods were all introduced in [43751], [43884], [43955] for the 5.0 branch, and then backported to trunk in [44116], [44261], [44281], respectively. So 56581-since.patch appears to be correct.

#6 @SergeyBiryukov
2 years ago

In 54193:

Docs: Correct @since tags for some block parser properties and methods.

This affects:

  • WP_Block_Parser::$empty_attrs
  • WP_Block_Parser::next_token()
  • WP_Block_Parser::freeform()
  • WP_Block_Parser_Block::$innerContent

The @since tags referred to early Gutenberg versions instead of WordPress core. These properties and methods were introduced in WordPress 5.0, so 5.0.0 is the correct version.

Some of the other @since tags are removed, as they are related to early Gutenberg development before it was merged into WordPress core, and are not relevant for core.

Follow-up to [43751], [43884], [43955] for the 5.0 branch, [44116], [44261], [44281] for trunk.

Props Chouby.
See #56581.

#7 in reply to: ↑ description ; follow-up: @SergeyBiryukov
2 years ago

Replying to Chouby:

WP_Block_Parser::parse() is documented as returning WP_Block_Parser_Block[] but it returns an array or arrays.

WP_Block_Parser::$output is also documented as being WP_Block_Parser_Block[] but is also an array of arrays.

This is visible in several places of the parse() method where $this->output is explicitely casted to an array.

Indeed. I believe we can use the WP_Block_Parser_Block[][] notation for that (with extra square brackets), there is a precedent for that in core as of [53058].

#8 in reply to: ↑ 7 @SergeyBiryukov
2 years ago

Replying to SergeyBiryukov:

I believe we can use the WP_Block_Parser_Block[][] notation for that (with extra square brackets), there is a precedent for that in core as of [53058].

Hmm, scratch that. Checking the type of WP_Block_Parser::$output, it is an array of arrays of various values and not an array of arrays of WP_Block_Parser_Block objects as I thought initially. 56581.patch is correct.

#9 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 54194:

Docs: Correct @return type for WP_Block_Parser::parse().

This affects:

  • WP_Block_Parser::parse()
  • WP_Block_Parser::$output

Both the method and the property are documented as returning WP_Block_Parser_Block[] (an array of WP_Block_Parser_Block objects), but the result is in fact an array of arrays of various values, so array[] is the correct notation.

Follow-up to [43751] for the 5.0 branch, [44116] for trunk.

Props Chouby.
Fixes #56581.

#10 @SergeyBiryukov
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It appears that these changes should first be made upstream in the Gutenberg respository and then backported to core, as the tests fail otherwise:

Ensure version-controlled files are not modified or deleted

Run git diff --exit-code
diff --git a/src/wp-includes/class-wp-block-parser.php b/src/wp-includes/class-wp-block-parser.php
index e987307..27c85cf 100644
--- a/src/wp-includes/class-wp-block-parser.php
+++ b/src/wp-includes/class-wp-block-parser.php
@@ -62,7 +62,7 @@ class WP_Block_Parser_Block {
 	 *   'innerContent' => array( 'Before', null, 'Inner', null, 'After' ),
 	 * )
 	 *
-	 * @since 5.0.0
+	 * @since 4.2.0
 	 * @var array
 	 */
 	public $innerContent;
...

#11 @SergeyBiryukov
2 years ago

In 54195:

Docs: Revert the WP_Block_Parser documentation changes now.

This commit reverts [54193] and [54194].

It appears that these changes should first be made upstream in the Gutenberg respository and then backported to core, as the tests fail otherwise.

See #56581.

#12 @SergeyBiryukov
2 years ago

Since this is a documentation-only change, I believe in can stay in 6.1 for now to be fixed during the beta cycle.

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


2 years ago

#14 @chaion07
2 years ago

@Chouby thanks for reporting this. We reviewed this ticket during a recent bug-scrub. Here's the summary of the feedback received from the team we are hoping to see this being merged after changes have been made in the Gutenberg Repo and then backported to Core. Thanks!

Props to @mukesh27 & @robinwpdeveloper

#15 @audrasjb
2 years ago

  • Milestone changed from 6.1 to 6.2

With WP 6.1 RC 1 scheduled today (Oct 11, 2022), there is not much time left to address this ticket. Let's move it to the next milestone.

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


22 months ago

#17 @costdev
22 months ago

This ticket was discussed during the recent bug scrub. I've opened an issue on the Gutenberg repository so that this can be looked at upstream: https://github.com/WordPress/gutenberg/issues/47947

@audrasjb Should this ticket be closed as reported-upstream, or should it be used to track the backport?

#18 @costdev
22 months ago

@SergeyBiryukov Take a look at @mamaduka's comment on the GH issue. It appears that the class is no longer maintained in the Gutenberg repository, and the error was unexpected.

Edit: It appears this was incorrect (a little to and fro never hurt anyone! 😅). See this comment on the GitHub issue.

Last edited 22 months ago by costdev (previous) (diff)

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


22 months ago

#20 @costdev
22 months ago

  • Milestone 6.2 deleted
  • Resolution set to reported-upstream
  • Status changed from reopened to closed

As this has been reported upstream and the change must happen there. I'm closing this ticket as reported-upstream.

Note: See TracTickets for help on using tickets.