WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 19 months ago

#17115 new defect (bug)

Publishing an empty post results in success

Reported by: kawauso Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.1
Component: Posts, Post Types Keywords: has-patch dev-feedback
Focuses: administration Cc:
PR Number:

Description

Steps to reproduce:

  1. Go to Add New post screen
  2. Publish post
  3. Success message is displayed with link to initial auto-draft of post

The issue can be traced back to the editpost switch case in post.php (line 204), which calls edit_post() and redirect_post().

edit_post() saves any meta values passed with the post to the auto-draft, which isn't necessarily desirable, and returns the auto-draft post ID.

redirect_post() then defaults to status message code 6 which incorrectly reports success in publishing.

Attachments (10)

disable-submit-inputs-meta-boxes.diff (3.4 KB) - added by eddieringle 8 years ago.
Initially disable submit inputs (meta-boxes.php)
disable-submit-inputs-post-js.diff (26.5 KB) - added by eddieringle 8 years ago.
Initially disable submit inputs via JS (post.js)
17115.diff (1.8 KB) - added by ericmann 6 years ago.
Simplified message patch.
17115.2.diff (2.5 KB) - added by adamsilverstein 5 years ago.
17115.3.diff (852 bytes) - added by adamsilverstein 5 years ago.
Cleaner, JS only solution
17115.4.diff (1.8 KB) - added by adamsilverstein 5 years ago.
disable publish button on load, re-enable on change
17115.4..diff (1.8 KB) - added by adamsilverstein 5 years ago.
17115.5.diff (1.1 KB) - added by GhostToast 4 years ago.
More careful check of $maybe_empty
17115.6.diff (1.2 KB) - added by GhostToast 4 years ago.
Ensure $maybe_empty is defined
17115.7.diff (4.5 KB) - added by adamsilverstein 3 years ago.

Download all attachments as: .zip

Change History (42)

#1 @chacha102
8 years ago

  • Keywords needs-patch added

It seems like this would be best solved in the front-end Javascript. Just a simple check that disables the Publish button until you've made a change to something on the page.

#2 @dd32
8 years ago

This sounds like it might even be a duplicate/new behaviour of #11082

@eddieringle
8 years ago

Initially disable submit inputs (meta-boxes.php)

#3 @eddieringle
8 years ago

  • Cc eddie@… added
  • Keywords has-patch added; needs-patch removed

Attached a diff that disables the submit buttons in the posting form. As soon as the user changes the contents of the title field and clicks off (causing the title field to lose focus), some background JS re-enables the submit inputs. (Just to clarify, all the diff does is initially disable the submit input elements).

#4 @dd32
8 years ago

By the look of that, the elements are disabled in HTML to start with? That will cause problems for non-js/accessible modes. Best bet would be to disable them via JS when loading the document if we took that route.

#5 @eddieringle
8 years ago

Ah, hadn't thought of that. Why don't we just validate the title and content fields and return a message if said validation fails?

@eddieringle
8 years ago

Initially disable submit inputs via JS (post.js)

#6 follow-up: @eddieringle
8 years ago

New diff disables the submit inputs through JS as suggested above. Appears to work nicely.

#7 in reply to: ↑ 6 @kawauso
8 years ago

Replying to eddieringle:

New diff disables the submit inputs through JS as suggested above. Appears to work nicely.

Good to see some life on this ticket. The new diff works fine for the HTML mode, but doesn't seem to work with TinyMCE / the visual editor. Also for future reference, you only need to provide a diff for the .dev version.

#8 @eddieringle
8 years ago

Works for me in both editors.

#10 @SergeyBiryukov
7 years ago

#22483 was marked as a duplicate.

#11 @trepmal
7 years ago

  • Cc trepmal@… added
  • Keywords needs-patch added; has-patch removed

Two primary issues I see with the JS method:

  1. Prematurely enables the update button if a non-crucial (title, content, excerpt) element is changed. For example, custom fields. (Mentioned in my dupe ticket #22483)
  2. Doesn't enable the button if only a featured image is added

I suspect there are also accessibility issues with a button that can only be used after being enabled (undisabled?) via javascript - but that is far from any area of expertise I may have.

The root of the problem seems to come from WordPress not allowing new posts without a title, content, and excerpt (http://core.trac.wordpress.org/browser/trunk/wp-includes/post.php#L2701).

While it may be correct for WordPress to not allow publishing a post without title/content, I think draft posts still need to be allowed. Someone may very well want to first upload and nice featured image and preview the post before adding anything else, and they cannot currently do so.

I'm not sure of the best way to go about patching this, but I'm going to tinker around and see what I can put together that makes any kind of sense to the user.

#12 @helen
6 years ago

Inaccurate admin notice is still an issue. There's a patch on #11082 that seems like it might be relevant to this problem. JS disable/enable of save and update buttons is not a complete fix here - I would call that part an enhancement and make another ticket for it.

@ericmann
6 years ago

Simplified message patch.

#13 follow-up: @ericmann
6 years ago

  • Keywords has-patch added; needs-patch removed

Ported a patch for this ticket, based on the existing work on #11082. Basically, check to see if the status of the "saved" post is still auto-draft. If so, it means things didn't save to the database and we should notify the author of that fact.

Just to clarify, there is a safeguard in core against adding posts with empty content. If the title, content, and excerpt are all empty, and the theme supports title, content, and excerpt for the post type, then wp_insert_post() will bail out early and nothing will be saved - this also means the save_post hook will not run. This functionality can be disabled with a hook:

add_filter( 'wp_insert_post_empty_content', '__return_false' );

See ~line 2670 of /wp-includes/post.php for details on the hook and what it's testing against.

#14 @helen
6 years ago

#25127 was marked as a duplicate.

#15 in reply to: ↑ 13 ; follow-up: @nerrad
6 years ago

Replying to ericmann:

Ported a patch for this ticket, based on the existing work on #11082. Basically, check to see if the status of the "saved" post is still auto-draft. If so, it means things didn't save to the database and we should notify the author of that fact.

Just to clarify, there is a safeguard in core against adding posts with empty content. If the title, content, and excerpt are all empty, and the theme supports title, content, and excerpt for the post type, then wp_insert_post() will bail out early and nothing will be saved - this also means the save_post hook will not run. This functionality can be disabled with a hook:

add_filter( 'wp_insert_post_empty_content', '__return_false' );

See ~line 2670 of /wp-includes/post.php for details on the hook and what it's testing against.

I saw that code but it appears that if title, content, and excerpt are all empty this bailout guard is NOT happening. Instead posts ARE getting saved to the db.

#16 in reply to: ↑ 15 @henrywright
6 years ago

Replying to nerrad:

I saw that code but it appears that if title, content, and excerpt are all empty this bailout guard is NOT happening. Instead posts ARE getting saved to the db.

I think the problem is they are never all empty. If you try removing everything in the content and hit save, you'll see some whitespace persists (even though you thought you removed all characters).

#17 @ocean90
6 years ago

#26753 was marked as a duplicate.

#18 @nacin
6 years ago

  • Component changed from Administration to Posts, Post Types
  • Focuses administration added

#19 @SergeyBiryukov
6 years ago

#27036 was marked as a duplicate.

#20 @frosso
6 years ago

#27556 was marked as a duplicate.

#21 @SergeyBiryukov
5 years ago

#29406 was marked as a duplicate.

#22 @adamsilverstein
5 years ago

In 17115.2.diff:

  • expand or @ericmann's patch, refresh against trunk - php warning useful if no-js
  • adds js side check before submitting form - if title, content and excerpt empty bail, don't submit

JS side check should prevent most empty submissions so maybe php warning not needed? also it seems wrong that the warning comes with a green color bar seeming to indicate success, it should really be a failure notice. De we want JS side notification, do we do that anywhere currently?

@adamsilverstein
5 years ago

Cleaner, JS only solution

#23 @adamsilverstein
5 years ago

  • Keywords dev-feedback added

17115.3.diff - slightly updated, JS only solution

  • Don't submit the form if title, content and excerpt are blank.

@adamsilverstein
5 years ago

disable publish button on load, re-enable on change

#24 @adamsilverstein
5 years ago

In 17115.4.diff:

  • Disable all buttons in the Publish metabox when the post-new.php page is loaded
  • Add callback on change/keyup for editor, title and excerpt, re-enabling publish buttons, and unhooking callbacks
  • Fix requires JS, no-js users get old behaviour

Buttons disabled on TinyMCE init:
http://f.cl.ly/items/3y443O1z3U0c0e0G0z3K/Add_New_Post__sadasd__WordPress_2015-01-28_14-09-24.jpg

Buttons re-enabled after any changes:
http://f.cl.ly/items/0n430g3u3k0G3v3D0d1k/Add_New_Post__sadasd__WordPress_2015-01-28_14-11-34.jpg

#25 @dnaber-de
5 years ago

I don't think that this issue can be solved with JavaScript. Such a solution would add only more restrictions to the UI. The filter wp_insert_post_empty_content explicit allows to override the default guard. With an additionally JavaScript inhibitor one would have to override two points to make empty posts publishable.

wp_insert_post() by default returns 0 when trying to publish an empty post via the Admin-UI, which is correct. The resulting message ID (/wp-admin/post.php?post=4&action=edit&message=6) is just wrong.

#26 @ocean90
5 years ago

#32134 was marked as a duplicate.

#27 @SergeyBiryukov
4 years ago

#35708 was marked as a duplicate.

#28 @GhostToast
4 years ago

To me this seems to be issue of the logic for how $maybe_empty is decided. If the post type in question doesn't support excerpt this doesn't fall down. My test is in trying to save taxonomy terms. If the title and content are empty, excerpt support is enabled, taxonomy terms don't save, despite a "Draft saved" message.

However if excerpt support is not enabled, $maybe_empty is false, and the post will save the terms.

Probably this should be failing on post types without excerpt support but the conditional is too long to bother considering whether a post has an Excerpt and whether it has support for one, so doesn't consider an empty Page, empty, for example.

@GhostToast
4 years ago

More careful check of $maybe_empty

#29 @GhostToast
4 years ago

Patch 17115.5 considers whether a given post supports a particular field before determining if it's presence matters. This will actually cause more things to be considered empty instead of just post types that have all 3 of these supports. It fixes the too-much-logic in one line that was happening when defining $maybe_empty.

In my own project, I ended up utilizing the filter to prevent my post type from being flagged as empty. This is important because much data could be saved in post meta or taxonomical connections, before a title, content or excerpt could be drafted. Looks a bit like this:

<?php
/**
 * Allow empty Review to be saved, even though it supports `excerpt`.
 * @param $maybe_empty
 * @param $postarr
 *
 * @return bool
 */
function slc_review_maybe_empty( $maybe_empty, $postarr ) {
        if (
                'slc_review' === $postarr['post_type']
                && empty( $postarr['post_title'] )
                && empty( $postarr['post_content'] )
                && empty( $postarr['post_excerpt'] )
        ) {
                return false;
        }
        return $maybe_empty;
}
add_filter( 'wp_insert_post_empty_content', 'slc_review_maybe_empty', 10, 2 );
Last edited 4 years ago by GhostToast (previous) (diff)

@GhostToast
4 years ago

Ensure $maybe_empty is defined

#30 @stephenharris
3 years ago

@GhostToast You raise a good point, $maybe_empty will only be set to true if the post type supports excerpt, title AND content. But that's not the issue here. A post does support all three of those. The issue here is that the error of an 'empty' post type is log inside edit_post() - it will always return the post ID, so the calling code can't detect an error to act accordingly.

Current patches take the approach of preventing the user of even being able to publish an 'empty' post. The problem with such an approach is as @dnaber-de describes.

#31 @adamsilverstein
3 years ago

17115.7.diff is a rollup of previous patches

  • When users publish a post with empty title/content/excerpt they get an error message saying they need to add content.
  • Saving post drafts with empty fields works fine and these still can't be published.
  • Respects the wp_insert_post_empty_content filter.

Screenshots


Publish a post with blank content fields: shows an error message, post is not saved:

https://cl.ly/1L1w0Y272v37/Screen%20Recording%202017-02-03%20at%2011.21%20AM.gif


Save a draft with blank content fields: success, draft is saved. Post still can't be published:

https://cl.ly/1m0K09181v02/Screen%20Recording%202017-02-03%20at%2011.51%20AM.gif


Adding the filter to enable publishing of blank posts works as expected. add_filter( 'wp_insert_post_empty_content', '__return_false' );

https://cl.ly/3S2v1v3o0p38/Screen%20Recording%202017-02-03%20at%2011.54%20AM.gif

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


19 months ago

Note: See TracTickets for help on using tickets.