Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#25272 closed task (blessed) (fixed)

Refactor/clean up autosave and use heartbeat for transport

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.8
Component: Autosave Keywords: early
Focuses: Cc:

Description

Autosave has lots of "moving parts" and over the years many additional functions were "pinned" to it. In 3.6 most of the additional functionality was moved to heartbeat, like logged out warning, nonces update, etc. It's time to move the saving of post data over to heartbeat as originally planned, and avoid the extra XHR.

Attachments (7)

25272.patch (42.7 KB) - added by azaozz 11 years ago.
25272.2.patch (42.7 KB) - added by azaozz 11 years ago.
25272-tests.patch (7.1 KB) - added by azaozz 11 years ago.
25272.3.patch (49.1 KB) - added by azaozz 10 years ago.
25272.4.patch (57.9 KB) - added by azaozz 10 years ago.
25272.5.patch (68.1 KB) - added by azaozz 10 years ago.
25272.6.patch (740 bytes) - added by azaozz 10 years ago.

Download all attachments as: .zip

Change History (41)

@azaozz
11 years ago

#1 @azaozz
11 years ago

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

In 25272.patch:

  • Remove wp_ajax_autosave().
  • Introduce autosave( $post_data ) (in PHP).
  • Remove the autosave nonce, use the edit-post . $post_id nonce instead.
  • Change post_status from auto-draft to draft in _wp_translate_postdata() (before the "What to do based on which button they pressed" part).
  • Change wp_create_post_autosave() to accept either the post data or post_id.
  • Update post_preview().
  • Refactor autosave.js:
    • Use heartbeat for transport.
    • Fix disabling and enabling of the Submit box buttons and links.
    • Fix behavior when users click "Preview".
    • Fix temporarily stopping of autosaves to avoid collisions.
    • Add wp.autosave.remote.disable() and wp.autosave.remote.triggerSave().
    • Add 'before-autosave' and 'after-autosave' custom events. Use the after-autosave event to load the edit slug part under the title field.
Last edited 11 years ago by azaozz (previous) (diff)

@azaozz
11 years ago

#2 @azaozz
11 years ago

25272.2.patch fixes a typo/missing arg in post_preview() in 25272.patch.

@azaozz
11 years ago

#3 @azaozz
11 years ago

In 25272-tests.patch: updated the autosave tests.

@azaozz
10 years ago

#4 @azaozz
10 years ago

In 25272.3.patch:

  • Move form submit handling, #title.blur(), etc. from autosave.js to post.js as they are only for that screen.
  • Rename 'wp.autosave.remote' to 'wp.autosave.server'.

This patch requires the heartbeat http://core.trac.wordpress.org/attachment/ticket/25073/25073.2.patch and includes the updated tests from 25272-tests.patch.

#5 @nacin
10 years ago

  • Keywords early added
  • Milestone changed from 3.7 to 3.8

This is huge. Per IRC, early 3.8. See also #23362 for 3.8.

#6 @westonruter
10 years ago

  • Cc weston@… added

#7 follow-up: @westonruter
10 years ago

@azaozz:

In your latest patch, could you make the following tweak:

- $('form#post').attr('target', 'wp-preview').submit().attr('target', ''); 
+ $('form#post').attr('target', $(this).attr('target')).submit().attr('target', '');

Two reasons for this. First, it eliminates redundancy as the wp-preview target is already defined in post_submit_meta_box() as:

<a class="preview button" href="<?php echo $preview_link; ?>" target="wp-preview" id="post-preview"><?php echo $preview_button; ?></a>

Secondly, I'm working on a plugin <https://github.com/x-team/wp-customizer-everywhere> that manipulates the window target for the preview, and the only way to do so at the moment is to override the global doPreview function with one that uses the above change. But if the #post-preview click handler supplies the $('form#post').attr('target') from its own target attribute, then a plugin just has to manipulate the preview link/button target upon jQuery.ready.

Last edited 10 years ago by westonruter (previous) (diff)

#8 in reply to: ↑ 7 @azaozz
10 years ago

Replying to westonruter:

This makes sense. Combined with the suggestion from #20233 about adding the post_id to the form submit target it should remove some confusion.

#9 @kadamwhite
10 years ago

  • Cc kadamwhite@… added

#10 @adamsilverstein
10 years ago

  • Cc adamsilverstein@… added

@azaozz
10 years ago

#11 @azaozz
10 years ago

In 25272.4.patch: refresh, pass autosave.js through jshint, make few names more specific.

#12 @azaozz
10 years ago

  • Milestone changed from 3.8 to Future Release

Moving to 3.9-early as discussed in the dev chat on 2013-11-20

The patch is quite large and could potentially introduce hard to spot bugs. 3.8 is even more condensed than 3.7, better to keep everybody's attention on the main features: MP6, Dashboard, Themes screen, Widgets...

@azaozz
10 years ago

#13 @azaozz
10 years ago

  • Milestone changed from Future Release to 3.9

In 25272.5.patch:

  • Improvements for wp.autosave.server.
  • Catch changes in both editors on beforeunload.
  • Refresh post.js.
Last edited 10 years ago by azaozz (previous) (diff)

#14 @azaozz
10 years ago

In 26995:

Autosave: refactor autosave.js, use heartbeat for transport and move all "Add/Edit Post" related functionality to post.js. See #25272.

#15 @azaozz
10 years ago

In 27003:

Autosave: don't disable the buttons on previewing a post, see #25272.

#16 @azaozz
10 years ago

In 27015:

Autosave: don't set "submit" buttons to disabled right before submitting the form. They are not sent with the form data, see #25272.

#17 @azaozz
10 years ago

In 27038:

Autosave:

  • Move the 'Saving post' and 'Draft saved at...' strings from autosaveL10n to postL10n as they are used only there.
  • Use the custom jQuery events 'before-autosave' and 'after-autosave' to show these messages.
  • Separate autosave.suspend() for local and server so local autosaves can continue while server autosaves are suspended.
  • Remove the recently added autosave.server.disable() and use autosave.server.suspend() instead.
  • Bring back .button.disabled, button-primary.disabled and use .disabled to prevent multiple form submissions.

See #25272.

#18 @iseulde
10 years ago

There's a typo in autosave.js on line 352: 'click.autosae-local'.
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/autosave.js?rev=27038#L352

#19 @nacin
10 years ago

We should probably set window.autosave = wp.autosave. Not sure if any plugins were using it to force an autosave, but there's little harm in doing so.

#20 @nacin
10 years ago

In 27092:

Autosave: Fix typo in event namespace. props avryl, see #25272.

#21 @azaozz
10 years ago

We should probably set window.autosave = wp.autosave...

Was looking at how many plugins do autosave = function(){} to disable it, may need to add back-compat for that.

@azaozz
10 years ago

#22 @azaozz
10 years ago

In 25272.6.patch: back-compat, before saving check whether the old window.autosave() was overridden by a plugin.

Last edited 10 years ago by azaozz (previous) (diff)

#23 @azaozz
10 years ago

Looks like there are eight plugins that disable autosave by overriding window.autosave(). Four are derivatives of AtD that will have to be updated anyway.

At this point I'm 50/50 on whether we need 25272.6.patch.

#24 @nacin
10 years ago

What was the canonical way in 3.8 to say "whoa there, don't run autosave"? Setting window.autosave to nothing? Maybe it was setting blockSave to true? I'm not sure what kind of back compat is needed here, but it seems like people surely came up with *some* way to block autosaves when necessary.

#25 @SergeyBiryukov
10 years ago

.button-disabled and .button-primary-disabled classes were removed from JS in [26995], [27035], and [27038] in favor of .button.disabled and button-primary.disabled. We can probably remove them from CSS too.

#26 @azaozz
10 years ago

What was the canonical way in 3.8 to say "whoa there, don't run autosave"?

Core used autosave = function(){}; mostly because jquery.schedule.js can't cancel existing schedule properly, it runs one more time. Some plugins picked that up too. Don't see any blockSave = true when searching the github mirror.

Speaking of jquery.schedule.js, it's not used anywhere in core any more. We need a way to deprecate scripts.

.button-disabled and .button-primary-disabled classes were removed from JS...

Thinking they are needed for back-compat.

#27 @kirasong
10 years ago

Is there a reason not to include the back-compat for overriding window.autosave()?
Since we're already defining it, it seems like not very much extra to do so.

On the second note, does it look like plugins are using .button-disabled and .button-primary-disabled?

#28 @nacin
10 years ago

In 27405:

Always convert auto-drafts to drafts in edit_draft() and _wp_translate_postdata().

This regressed due to refactoring in [26995]. This commit fixes Quick Draft.

see #25272.

#29 @nacin
10 years ago

  • Type changed from enhancement to task (blessed)

#30 @nacin
10 years ago

In 27611:

Improve the autosave error message on nonce failure.

see #27453, #25272.

#31 @nacin
10 years ago

In 27612:

Remove two new strings from post_preview().

see #25272, #27453.

#32 @nacin
10 years ago

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

In 27713:

Back compat for overriding window.autosave.

props azaozz.
fixes #25272.

#33 @nacin
10 years ago

New tickets for new issues. For example, #27503.

#34 @azaozz
10 years ago

In 27981:

Heartbeat: no longer "experimental", see #25272

Note: See TracTickets for help on using tickets.