Skip to content
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

Default title and content are not reflected in new editing. #8757

Closed
tmatsuur opened this issue Aug 9, 2018 · 11 comments
Closed

Default title and content are not reflected in new editing. #8757

tmatsuur opened this issue Aug 9, 2018 · 11 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Extensibility The ability to extend blocks or the editing experience REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended

Comments

@tmatsuur
Copy link

tmatsuur commented Aug 9, 2018

I posted it to Trac and it was guided to here.

I am using the 'default_content' filter on new postings to implement replication of posts.
As I tried with Gutenberg's new editor, the 'default_content' filter is working, but the content part of the editor is empty.
For new posts, 'get_default_post_to_edit' function first saved empty post data, then changed the content with the 'default_content' filter, and the new editor is loading empty post data.

Experimentally, set 'Hello' to post_title, set 'World' to post_content and updated post data after 'default_title' filter.
As a result, the title of the new editor was left empty, but 'World' was reflected in the content part.

I thought that the 'default_content' and 'default_title' filters are not working properly with the new editor.

@designsimply designsimply added [Type] Bug An existing feature does not function as intended [Feature] Extensibility The ability to extend blocks or the editing experience Needs Testing Needs further testing to be confirmed. labels Aug 9, 2018
@Chouby
Copy link
Contributor

Chouby commented Aug 20, 2018

It's true for the default_excerpt filter too.

The whole issue is easy to test with:

add_filter( 'default_title', function() {
	return 'My default title';
} );

add_filter( 'default_content', function() {
	return 'My default content';
} );

add_filter( 'default_excerpt', function() {
	return 'My default excerpt';
} );

@brandonpayton
Copy link
Member

Tested and confirmed that Gutenberg doesn't consider these filters or at least their output.

@brandonpayton brandonpayton removed the Needs Testing Needs further testing to be confirmed. label Sep 26, 2018
@danielbachhuber danielbachhuber added the Backwards Compatibility Issues or PRs that impact backwards compatability label Oct 4, 2018
@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone Oct 4, 2018
@danielbachhuber
Copy link
Member

If there's not an obvious place for this in Gutenberg, we may actually want to apply these filters within the REST API, but only when a new post is created.

@danielbachhuber danielbachhuber added the REST API Interaction Related to REST API label Oct 4, 2018
@mnelson4
Copy link

mnelson4 commented Oct 4, 2018

Looking into this, I think by the time the REST API comes into play, it's too late to change the editor's initial content. The initial draft post was created upon loading post-new.php, and get_default_post_to_edit( $post_type, true ); is actually called as normal (which saves the draft post).
The trouble is when gutenberg_init() is called, it apparently ignores the $post object passed into it, so the page loads with empty data.
POST wp-json/wp/v2/posts/{id}/autosaves is called only later when the post autosaves (and the user has already started editing the page). So that's not only a bit late, but even if we apply those filters inside WP_REST_Autosaves_Controller::create_item(), the editor ignores those changes (treating its content as the authority).
POST wp-json/wp/v2/posts/{id} is only called even later when publishing.

So far, I'm unable to find the place where the Gutenberg code is erroneously assuming the $post's content is blank

@Chouby
Copy link
Contributor

Chouby commented Oct 5, 2018

I don't believe that the issue is coming from the REST API which in my opinion works as it should. I rather believe that the issue is coming from the way Gutenberg is handling the new post. Bullets 2 and 3 of my #7000 (comment) explain the issues.

As @mnelson4 is writing, the new post is created as usual by get_default_post_to_edit() and stored in the global $post. It's important to notice that at this point WordPress doesn't save the default content in the database. It creates an empty post and fills the global $post with the default content. There is indeed no need to save the default content in database as WordPress uses directly the global $post.

But Gutenberg doesn't use the global $post and reads the post from the database via the REST API. Since that post is empty (and was created just to get an ID), the default content is screwed up by Gutenberg.

The solution that I proposed in #7421 with a filter specific to Gutenberg is probably not the best solution.

I see two possibilites:

  1. Saving the global $post just before the REST API call there https://github.com/WordPress/gutenberg/blob/v3.9.0-rc.1/lib/client-assets.php#L1296. Then we will of course need to remove the specific code for the title there: https://github.com/WordPress/gutenberg/blob/v3.9.0-rc.1/lib/client-assets.php#L1341-L1354
  2. Use the same approach currently used for the demo and the the title in https://github.com/WordPress/gutenberg/blob/v3.9.0-rc.1/lib/client-assets.php#L1320-L1354 but filling the default title, content and excerpt from the existing values from $post instead of what currently done. This is bit more code but avoids a database update.

I am not sure that the demo content will land in WordPress 5.0. If that's the case, I would suggest to use the existing default_title and default_content filters rather than the current approach.

@danielbachhuber what's your opinion about the 2 proposed solutions?

@danielbachhuber
Copy link
Member

To be perfectly honest, I don't love either of those possibilities. Both seem relatively hacky.

To throw a third and fourth option into the mix:

  1. Introduce default_title, default_excerpt, default_contentJavaScript filters that perform equivalent behavior. While this wouldn't maintain 100% backwards compatibility, it would give existing plugins and themes a more obvious upgrade path for Gutenberg support. Filter added to 'page_attributes_dropdown_pages_args' isn't applied in Gutenberg, causing page parent relationships to change #9089 is a related scenario that will probably be handled this way.
  2. More precisely replicate the existing core behavior of get_default_post_to_edit() by applying the default_title, default_excerpt, default_content filters to the REST API response. Notably, get_default_post_to_edit() also respects $_REQUEST['post_title'] so we might as well handle that too.

I'm leaning towards option 4 right now. We could achieve this by modifying the global $post when setup_postdata() is called within prepare_item_for_response(). To limit the scope of what this callback would modify, we'd want to separate out the preload API request for https://github.com/WordPress/gutenberg/blob/v3.9.0-rc.1/lib/client-assets.php#L1286, add the callback immediately before, and remove it immediately after.

@mnelson4
Copy link

mnelson4 commented Oct 5, 2018

I made a PR of Option 2. It actually was hardly any code at all and doesn't seem hacky to me. When the PHP code is populating the form inputs, just use the values already set on the available $post, instead of assuming they're all blanks.
Like @Chouby suggested, the demo content could instead be refactored to use the existing default_content etc, but that could be a separate issue.

Something that needs to be clarified: when visiting the post-new.php with Gutenberg, there is no call to GET wp-json/wp/v2/posts. The initial content is entirely populated server-side. (See https://github.com/WordPress/gutenberg/pull/10362/files).

Re: Option 1: there is no need to do that save, we can use the data already set on the post (which is what my PR does).
Re: Option 3: that's possible, but those filters would need to take the value already set server-side and change it. The existing filters already changed the defaults, I don't see why it would be better to continue ignoring those server-side-set defaults, and then require adding new javascript-side filters.
Re: Option 4: My PR actually currently supports setting ?post_title=title-from-querystring&content=content-from-querystring. Eg
gutenberg defaults from querystring is using my PR's branch. What's more, that won't actually fix this issue because, as noted, when loading the post-new.php, there is NO REST API request to fetch the current post (that I can tell; maybe I'm overlooking something).
Having said that, maybe option 4 would still be good for other applications. But it doesn't appear it will solve this problem for Gutenberg.

Does that make sense or am I totally out to lunch?

@danielbachhuber
Copy link
Member

Does that make sense or am I totally out to lunch?

Seems fine to me!

@Chouby Does this address your issue?

@Chouby
Copy link
Contributor

Chouby commented Oct 8, 2018

#10362 Looks good to me. This is my preferred solution too.

@mnelson4
Copy link

mnelson4 commented Oct 8, 2018

👍 thanks @danielbachhuber @Chouby. Now to see if we can get that PR landed...

@danielbachhuber
Copy link
Member

Fixed with #10362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Extensibility The ability to extend blocks or the editing experience REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants