Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#18227 closed defect (bug) (fixed)

Autosave JS throws and error for post types with disabled title and editor

Reported by: karevn's profile karevn Owned by: azaozz's profile azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: Autosave Keywords: has-patch
Focuses: Cc:

Description

Steps to reproduce:

  • Register a custom post type with title and editor disabled
  • Create a post of that type
  • Open the editor screen, wait for about a minute for autosave to try saving the post.

An error will be thrown because there is no post_datapost_title? set.

Attachments (2)

autosave.patch (777 bytes) - added by karevn 13 years ago.
autosave.dev.js.patch (687 bytes) - added by MarcusPope 13 years ago.
Improved patch recommendation by Marcus Pope

Download all attachments as: .zip

Change History (10)

@karevn
13 years ago

#1 @MarcusPope
13 years ago

I'm pretty sure this will never be true unless post_data["content"] is an array or custom js object with a namespace-trumping length property:

(post_data["content"] && post_data["content"].length == 0)

Instead we could just check for !post_data[xxx] on line 259:

if ( (!post_data["post_title"] && !post_data["content"]) || 
       post_data["post_title"] + post_data["content"] == autosaveLast ) {

Thanks,
Marcus

#2 follow-up: @azaozz
13 years ago

All autosave cares about is the post_title and content. There's no point in trying to do a save if they don't exist, so setting doAutoSave = false; and skipping all the parts dealing with title or content should be enough.

#3 @azaozz
13 years ago

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

#4 in reply to: ↑ 2 @MarcusPope
13 years ago

Replying to azaozz:

My fix is intended purely to prevent the JS error that is thrown when editor or title support are removed from a post - a fix of that nature should definitely happen regardless of autosave's purpose.

However I also think preventing autosave entirely because those fields were not enabled or because they are empty seems like a poor assumption - especially so with the advent of custom post types. Why shouldn't a custom post type be allowed to rely on the autosave functionality for custom meta properties it has implemented in a save_post action hook?

Ideally this code would be refactored entirely to check if each feature is enabled first before relying on empty post variables as a trigger to skip the autosave process.

But even outside of custom post types, it would still suck to write an entire blog post and lose all progress solely because you didn't populate the post's title first.

Last edited 13 years ago by MarcusPope (previous) (diff)

@MarcusPope
13 years ago

Improved patch recommendation by Marcus Pope

#6 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 3.3

#7 @azaozz
12 years ago

Seems ticket:19390:19390.diff would work best for now. To extend autosave for CPTs and (perhaps) other instances of the editor we will need to refactor it.

#8 @azaozz
12 years ago

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

In [19476]:

Prevent error in autosave for CPTs without title or editor, props sorich87, fixes #18227

Note: See TracTickets for help on using tickets.