WordPress.org

Make WordPress Core

#24756 closed defect (bug) (fixed)

Don't limit local autosave only to Posts

Reported by: azaozz Owned by: azaozz
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Autosave Keywords: has-patch
Focuses: Cc:

Description

There is some left-over code limiting the local autosave (in sessionStorage) only to Posts. This limitation should be removed making it run on all Add New* and Edit* screens where autosave.js is loaded or alternatively it should also run on Add New Page and Edit Page.

Attachments (11)

24756.patch (741 bytes) - added by azaozz 20 months ago.
Enable on all screens where autosave.js is loaded
24756-2.patch (711 bytes) - added by azaozz 20 months ago.
Enable on Add/Edit Post and Add/Edit Page only
24756-3.patch (871 bytes) - added by azaozz 20 months ago.
24756.diff (859 bytes) - added by nacin 20 months ago.
24756-4.patch (4.4 KB) - added by azaozz 20 months ago.
24756-5.patch (4.7 KB) - added by azaozz 20 months ago.
24756-6.patch (6.3 KB) - added by azaozz 20 months ago.
24756-7.patch (11.0 KB) - added by azaozz 20 months ago.
24756-8.patch (12.3 KB) - added by azaozz 20 months ago.
24756.2.diff (9.9 KB) - added by nacin 20 months ago.
24756.3.diff (23.0 KB) - added by nacin 20 months ago.
Same as 24756.2.diff, with max context.

Download all attachments as: .zip

Change History (33)

@azaozz20 months ago

Enable on all screens where autosave.js is loaded

@azaozz20 months ago

Enable on Add/Edit Post and Add/Edit Page only

comment:1 @nacin20 months ago

All screens plz. :-)

Let's make sure everything works okay with a post type doesn't support title, content, or excerpt; and doesn't support any of those.

comment:2 @nacin20 months ago

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

azaozz is looking into this and trying to make sure it is fully resilient.

@azaozz20 months ago

comment:3 @azaozz20 months ago

In 24756-3.patch: add a check whether the post type supports either editor or excerpt.

comment:4 @nacin20 months ago

  • Keywords has-patch added

Patch looks good at a glance.

comment:5 @azaozz20 months ago

Patch looks good at a glance.

Was passing the CPT 'supports' array to JS to detect when there is no support for editor and excerpt, but 24756-3.patch does the same in a much simpler way.

comment:6 @nacin20 months ago

I agree.

@nacin20 months ago

comment:7 @nacin20 months ago

In 24747:

Post type support for local autosaves. props azaozz. see #24756.

comment:8 @nacin20 months ago

  • Owner changed from azaozz to nacin

Leaving this open for azaozz who is removing the editor.js dependency from autosave.js. editor.js is tied to the admin, while autosave.js can actually be used on the frontend.

comment:9 @nacin20 months ago

  • Owner changed from nacin to azaozz

@azaozz20 months ago

comment:10 @azaozz20 months ago

In 24756-4.patch:

  • Remove autosave.js dependency on editor.js.
  • Move the check whether the loaded post content is different than local autosave post content to run on DOM ready.

@azaozz20 months ago

comment:11 @azaozz20 months ago

24756-5.patch also includes @carldanley's patch for using hasConnectionError() when enabling buttons.

comment:12 @nacin20 months ago

I don't entirely follow all of the moves in 24756-5.patch but it seems to work and seems to look good.

comment:13 @aaroncampbell20 months ago

24756-5.patch seems to work for me. I'm not sure if there are any specific edge cases I should be testing though.

comment:14 @bradparbs20 months ago

24756-5.patch​ works for me, too.

Looks good.

comment:15 @bradparbs20 months ago

  • Cc brad@… added

@azaozz20 months ago

comment:16 @azaozz20 months ago

24756-6.patch is same as 24756-5.patch plus: always tracks changes in the excerpt and removes one unused var.

@azaozz20 months ago

comment:17 @azaozz20 months ago

24756-7.patch includes 5.patch and 6.patch plus:

  • Output the local_storage_notice HTML on all Add/Edit screens.
  • Introduce wp.autosave.getLastSavedCompareString().
  • Minor cleanup of autosave(), removed doAutoSave as we don't connect when it's false.

@azaozz20 months ago

comment:18 @azaozz20 months ago

24756-8.patch is same as 7.patch except it moves _local_storage_notice() to wp-admin/includes/template.php

comment:19 @nacin20 months ago

In 24762:

Move _local_storage_notice() to admin/includes/template.php. props azaozz, see #24756.

@nacin20 months ago

comment:20 @nacin20 months ago

24756.2.diff is 24756-8.patch minus [24762]. I've reviewed it and it looks good. Because there are so many moving bits here, we just need to make sure everything gets tested well.

I think getLastSavedCompareString() is a great abstraction. I'm finding that its name implies it is getting the last saved string, not a string that will then be compared against whatever is saved. I think maybe just wp.autosave.getCompareString() is fine.

@nacin20 months ago

Same as 24756.2.diff, with max context.

comment:21 @nacin20 months ago

if ( ( post_data["post_title"].length == 0 && post_data["content"].length == 0 ) ... return is now missing. Seems like given the proper comparisons, it is no longer needed? Going to commit.

comment:22 @nacin20 months ago

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

In 24774:

Autosave:

  • Remove editor.js as a dependency for autosave.js, as it was in 3.5.
  • Remove heartbeat.js as an implicit dependency.
  • Abstract out the serialization of title/content/excerpt for comparisons.

props azaozz.
fixes #24756.

Note: See TracTickets for help on using tickets.