Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#47216 closed defect (bug) (fixed)

Block Editor crashes on custom post types without title support

Reported by: markjaquith's profile markjaquith Owned by: audrasjb's profile audrasjb
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Editor Keywords:
Focuses: javascript Cc:

Description

Create a custom post type that supports 'editor' but not 'title'. Save Draft. Reload the page. Crashes.

<?php
add_action( 'init', function () {
        register_post_type( 'movie',
                [
                        'labels' => [
                                        'name' => 'Movies',
                                        'singular_name' => 'Movie',
                        ],
                        'public' => true,
                        'has_archive' => true,
                        'rewrite' => [
                                'slug' => 'movies'
                        ],
                        'show_in_rest' => true,
                        'supports' => [
                                //'title',
                                'editor',
                        ],
                ]
        );
});

Attachments (2)

47216.1.patch (452 bytes) - added by splitti 5 years ago.
Patches cleanForSlug to not fail when the parameter isn't a string.
47216.2.patch (461 bytes) - added by splitti 5 years ago.
Updated patch that follows the coding standards

Download all attachments as: .zip

Change History (18)

This ticket was mentioned in Slack in #core-editor by earnjam. View the logs.


6 years ago

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


6 years ago

#3 @desrosj
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.3
  • Priority changed from normal to high

#4 @splitti
5 years ago

  • Keywords has-patch added; needs-patch removed

The problem is caused by this line in PostLink.

It assumes that cleanForSlug (see here) can deal with parameters that aren't strings. If this were a documentary, the narrator would now say "It can't."

There are at least two options on how to resolve this problem.

  1. Make cleanForSlug bail whenever it gets invalid input.
  2. Change the code in PostLink (and potentially elsewhere) to not call cleanForSlug with something that isn't always a string.

Following the Robustness Principle, I opted for (1) in my patch.
I'm not sure what's the best way to indicate that the function wasn't able to perform the desired action, but I think returning undefined is probably a good start, but I'd like to hear your opinions on that as well as any preference for another option :)

@splitti
5 years ago

Patches cleanForSlug to not fail when the parameter isn't a string.

This ticket was mentioned in Slack in #core-js by splitti. View the logs.


5 years ago

@splitti
5 years ago

Updated patch that follows the coding standards

#6 @splitti
5 years ago

Adjusted the patch to follow the coding standards :)

This ticket was mentioned in Slack in #core-editor by chrisvanpatten. View the logs.


5 years ago

#8 @SergeyBiryukov
5 years ago

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

The patch has been merged in https://github.com/WordPress/gutenberg/pull/16236 and should be included in core the next time Gutenberg packages are merged.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#9 @JeffPaul
5 years ago

  • Keywords needs-testing fixed-major added
  • Milestone changed from 5.3 to 5.2.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this so it can be back-ported to the 5.2 branch, also needs testing to validate if this ticket is good to land in 5.2.3.

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


5 years ago

#11 @audrasjb
5 years ago

  • Keywords close added

@JeffPaul Looks like it was fixed in Gutenberg so I guess there is nothing to backport here.
https://github.com/WordPress/gutenberg/pull/16236/files/cd82bed57c1f3722460d93438148b0edd92e3892

Asking in #core-editor for confirmation :)

#12 @audrasjb
5 years ago

  • Owner set to audrasjb
  • Status changed from reopened to reviewing

This ticket was mentioned in Slack in #core-editor by audrasjb. View the logs.


5 years ago

#14 @audrasjb
5 years ago

  • Keywords has-patch needs-testing fixed-major close removed
  • Milestone 5.2.3 deleted
  • Priority changed from high to normal
  • Resolution set to reported-upstream
  • Status changed from reviewing to closed

I asked in #core-editor and the answer was clear:

While I agree it would be good to fix the process to update the packages in Core is still not that straightforward. So given that we'll be focusing on WordPress 5.3 a lot in the next weeks and the timeline for the feature-freeze being in a month or so. I'd prefer if we just land this in WordPress 5.3 due to that overheard.

Unfortunately, the Gutenberg patch (already merged) is not going to land in 5.2.3 but it the issue will definitely be fixed in 5.3.

By the way, I'm going to close the ticket as reported-upstream since it's already fixed in Gutenberg.

#15 @audrasjb
5 years ago

  • Milestone set to 5.3

#16 @SergeyBiryukov
5 years ago

  • Resolution changed from reported-upstream to fixed
Note: See TracTickets for help on using tickets.