Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#42502 closed defect (bug) (fixed)

Customize: Autosave revision not dismissed after first change

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Customize Keywords: has-patch commit dev-reviewed
Focuses: Cc:

Description

After having saved a draft of a changeset, any changes you make after this will get stored in an autosave revision for that changeset. If you reload the Customizer without saving the changeset again, then when the Customizer is loaded a notification will appear for whether or not to restore the autosave revision. If the user clicks the dismiss button, then an Ajax request is opened to dismiss the autosave to prevent it from being prompted again and the notification is removed. Additionally, the notification is removed if any change is subsequently made, but the autosave revision dismissal request is not opened in this case.

See code here: https://github.com/WordPress/wordpress-develop/blob/58fad5b/src/wp-admin/js/customize-controls.js#L8297-L8344

The lack of autosave revision dismissal at the point where a change happens has a negative consequence: an Ajax request in the preview will include the customize_autosaved parameter (since the autosaving message was received, see #42475) but since the autosave revision hasn't been dismissed then it will get loaded in the request, and cause a mismatch between the changeset state in the user's browser and the changeset data that is used for previewing.

The easiest way to replicate the problem is:

  1. Add 5 nav menu items
  2. Save draft
  3. Put the last item as the first and the first item as the last.
  4. Reload, giving a second before confirming to give autosave a chance.
  5. Notice the autosave notification.
  6. Move the second nav menu item after the third nav menu item.
  7. Notice the autosave notification go away.
  8. BUG: Notice that even though you only moved the second nav menu item after the third, the first and last nav menu items _also_ switched places. This is because the autosave revision was incorrectly restored since it hasn't been dismissed yet.

Video: https://youtu.be/dnu8Lw8Pr14

The immediate fix is just to make sure the revision dismissal request happens both when dismissing the notification explicitly and when dismissal happens implicitly as result changing an item.

In 4.9.1 I'd like to revisit the autosaving logic to harden how the autosaved state is passed around.

Attachments (2)

42502.diff (1.6 KB) - added by westonruter 7 years ago.
42502.2.diff (1.9 KB) - added by westonruter 7 years ago.
Prevent a second unnecessary request to dismiss autosave revision after explicit dismissal (via X click) is followed by implicit dismissal (via change event)

Download all attachments as: .zip

Change History (12)

@westonruter
7 years ago

#1 @westonruter
7 years ago

  • Keywords has-patch needs-testing added

#2 @melchoyce
7 years ago

Able to replicate the bug in trunk.

Testing your patch, however, the issue persists. :/ I'm still seeing the menu items switch places.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


7 years ago

#4 @westonruter
7 years ago

  • Keywords dev-feedback commit added; needs-testing removed

#5 @westonruter
7 years ago

Here is another video, with narration, that demonstrates the issue that the patch is intended to fix: https://youtu.be/odlb-h0WPNw

The patch makes sure that the autosave revision gets dismissed whether the notification is dismissed explicitly (with a click on the notifications X button) or implicitly (by making any change in a control).

#6 @westonruter
7 years ago

Here is another video, with narration, that demonstrates the issue that the patch is intended to fix: https://youtu.be/odlb-h0WPNw

The patch makes sure that the autosave revision gets dismissed whether the notification is dismissed explicitly (with a click on the notifications X button) or implicitly (by making any change in a control).

#7 @johnbillion
7 years ago

  • Keywords dev-feedback removed

I was able to reproduce the problem, and after applying 42502.diff the problem no longer appears.

For other testers: The problem doesn't manifest itself in the customizer controls, only in the preview pane, resulting in correctly ordered menu items in the customizer control pane but incorrectly ordered menu items in the preview.

#8 @johnbillion
7 years ago

  • Keywords dev-reviewed added

Good to go

@westonruter
7 years ago

Prevent a second unnecessary request to dismiss autosave revision after explicit dismissal (via X click) is followed by implicit dismissal (via change event)

#9 @westonruter
7 years ago

  • Owner set to westonruter
  • Resolution set to fixed
  • Status changed from new to closed

In 42144:

Customize: Ensure autosave revision is dismissed immediately after implicit restoration notice dismissal as done with explicit notice dismissal.

Fixes issue where a drafted/scheduled changeset could inadvertently re-use the previous autosave revision in the preview while a user expects it to have been dismissed.

See #39896, [41597].
Fixes #42502 for trunk.

#10 @westonruter
7 years ago

In 42145:

Customize: Ensure autosave revision is dismissed immediately after implicit restoration notice dismissal as done with explicit notice dismissal.

Fixes issue where a drafted/scheduled changeset could inadvertently re-use the previous autosave revision in the preview while a user expects it to have been dismissed.

See #39896, [41597].
Fixes #42502 for 4.9.

Note: See TracTickets for help on using tickets.