WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 3 weeks ago

Last modified 11 days ago

#25272 closed task (blessed) (fixed)

Refactor/clean up autosave and use heartbeat for transport

Reported by: azaozz Owned by: 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 7 months ago.
25272.2.patch (42.7 KB) - added by azaozz 7 months ago.
25272-tests.patch (7.1 KB) - added by azaozz 7 months ago.
25272.3.patch (49.1 KB) - added by azaozz 7 months ago.
25272.4.patch (57.9 KB) - added by azaozz 5 months ago.
25272.5.patch (68.1 KB) - added by azaozz 3 months ago.
25272.6.patch (740 bytes) - added by azaozz 2 months ago.

Download all attachments as: .zip

Change History (41)

azaozz7 months ago

comment:1 azaozz7 months 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 7 months ago by azaozz (previous) (diff)

azaozz7 months ago

comment:2 azaozz7 months ago

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

azaozz7 months ago

comment:3 azaozz7 months ago

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

azaozz7 months ago

comment:4 azaozz7 months 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.

comment:5 nacin7 months 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.

comment:6 westonruter7 months ago

  • Cc weston@… added

comment:7 follow-up: westonruter7 months 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 7 months ago by westonruter (previous) (diff)

comment:8 in reply to: ↑ 7 azaozz6 months 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.

comment:9 kadamwhite5 months ago

  • Cc kadamwhite@… added

comment:10 adamsilverstein5 months ago

  • Cc adamsilverstein@… added

azaozz5 months ago

comment:11 azaozz5 months ago

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

comment:12 azaozz5 months 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...

azaozz3 months ago

comment:13 azaozz3 months 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 3 months ago by azaozz (previous) (diff)

comment:14 azaozz3 months ago

In 26995:

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

comment:15 azaozz3 months ago

In 27003:

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

comment:16 azaozz3 months 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.

comment:17 azaozz3 months 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.

comment:18 avryl2 months 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

comment:19 nacin2 months 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.

comment:20 nacin2 months ago

In 27092:

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

comment:21 azaozz2 months 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.

azaozz2 months ago

comment:22 azaozz2 months ago

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

Last edited 2 months ago by azaozz (previous) (diff)

comment:23 azaozz2 months 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.

comment:24 nacin2 months 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.

comment:25 SergeyBiryukov2 months 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.

comment:26 azaozz2 months 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.

comment:27 DH-Shredder6 weeks 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?

comment:28 nacin6 weeks 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.

comment:29 nacin6 weeks ago

  • Type changed from enhancement to task (blessed)

comment:30 nacin4 weeks ago

In 27611:

Improve the autosave error message on nonce failure.

see #27453, #25272.

comment:31 nacin4 weeks ago

In 27612:

Remove two new strings from post_preview().

see #25272, #27453.

comment:32 nacin3 weeks ago

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

In 27713:

Back compat for overriding window.autosave.

props azaozz.
fixes #25272.

comment:33 nacin3 weeks ago

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

comment:34 azaozz11 days ago

In 27981:

Heartbeat: no longer "experimental", see #25272

Note: See TracTickets for help on using tickets.