WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

Last modified 4 months ago

#39227 closed defect (bug) (fixed)

Changeset parameter not generated

Reported by: pavelevap Owned by: westonruter
Milestone: 4.7.3 Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: commit has-patch fixed-major
Focuses: Cc:

Description

I tried this new feature with Twenty Seventeen, went to Customizer, changed Color schema in left column and there is no changeset URL parameter to copy and share?

I waited a little bit, but nothing happened until I clicked somewhere into preview area. Then, changeset parameter was finally generated. I do not think it is expected behaviour?

Attachments (3)

changeset_uuid-param-added-upon-window-blur.mov (2.0 MB) - added by westonruter 8 months ago.
39227.diff (503 bytes) - added by asalce 8 months ago.
39227.2.diff (539 bytes) - added by westonruter 8 months ago.

Change History (21)

#1 @westonruter
8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7.1
  • Owner set to westonruter
  • Status changed from new to accepted

The changes get persisted into a changeset currently at the AUTOSAVE_INTERVAL or whenever you blur the customize.php window (or beforeunload it). This is why you see the changeset_uuid appear in the URL the moment that you click into the preview, because this is a blur event on the customize.php window. This is probably an unintentional side effect of the fix done in #39081. This needs to be fixed.

However, when starting customization with starter content initially supplied, I'm not seeing the URL update as expected. I am seeing this behavior once fresh_site is 0. Specifically, once you click into the URL bar you should see the changeset_uuid param supplied, as in changeset_uuid-param-added-upon-window-blur.mov.

Nevertheless, we really shouldn't be waiting for the customize_changeset post to be created before updating the URL with the changeset_uuid param. The moment that the saved state becomes false we should at that moment make sure that the changeset_uuid param is added to the URL.

#2 @pavelevap
8 months ago

Yes, you are right, I did not try to click URL bar when I did not see that there is no changeset_uuid :-) When i click URL, parameter appears (with a small delay).

@asalce
8 months ago

#3 @asalce
8 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

I was able to duplicate the issue on latest nightly release and 4.7 . I attached a patch, let me know if that works for ya!

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 months ago

#5 follow-up: @adamsilverstein
8 months ago

  • Keywords needs-testing removed

@asalce Nice work! I tested 39227.diff and it fixes the issue.

Here is a screencast showing the issue, before and after the patch for the record:

before: https://cl.ly/1M2j1b2A3J1N - the uuid is not added initially
after: https://cl.ly/3p2W1d0H470b - the uuid is added immediately

@westonruter I notice when I hit refresh I get the "Changes you made may not be saved." warning modal... is that still true? with the changesets, when i hit refresh my changes aren't lost, right?

#6 in reply to: ↑ 5 @westonruter
8 months ago

Replying to adamsilverstein:

I notice when I hit refresh I get the "Changes you made may not be saved." warning modal... is that still true? with the changesets, when i hit refresh my changes aren't lost, right?

It's not strictly true but it it may be true in practice because there is no UI or regaining access to the auto-draft changeset. The in the UI being developed in the Customize Snapshots plugin, upon saving an actual explicit Draft of the changes the AYS dialog in that case will not surface.

#7 @westonruter
8 months ago

  • Keywords needs-patch added; has-patch removed

I also updated the Customizer Browser History plugin to implement a related fix: https://github.com/xwp/wp-customizer-browser-history/pull/16

@westonruter
8 months ago

#8 @westonruter
8 months ago

  • Keywords commit has-patch added; needs-patch removed

#9 @westonruter
8 months ago

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

In 39686:

Customize: Update customize.php URL with changeset_uuid param the instant a change is made instead of deferring until the changeset update request responds.

Props asalce.
Fixes #39227.

#10 @westonruter
8 months ago

  • Keywords fixed-major added

Re-opening for 4.7.1

#11 @asalce
8 months ago

@westonruter @adamsilverstein no problem! Glad I could help.

#12 @pavelevap
7 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It is closed for 4.7.1 milestone, but there is no commit for 4.7 branch?

#13 @ocean90
7 months ago

  • Milestone changed from 4.7.1 to 4.7.2

Right, looks like @westonruter missed to actually re-open the ticket in comment:10.

Moving to 4.7.2 since 4.7.1 is already in the pipe.

#14 @westonruter
7 months ago

Sorry 😞

#15 @dd32
6 months ago

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

In 40093:

Customize: Update customize.php URL with changeset_uuid param the instant a change is made instead of deferring until the changeset update request responds.

Props asalce, westonruter.
Merges [39686] to the 4.7 branch.
Fixes #39227.

#16 @westonruter
4 months ago

This caused a regression in IE9. See #40405.

#17 @westonruter
4 months ago

In 40405:

Customize: Verify availability of history.replaceState (in IE9) before attempting to populate changeset_uuid parameter.

Props westonruter, timmydcrawford for testing.
Amends [39686].
See #39227.
Fixes #40405.

#18 @swissspidy
4 months ago

In 40420:

Customize: Verify availability of history.replaceState (in IE9) before attempting to populate changeset_uuid parameter.

Props westonruter, timmydcrawford for testing.
Amends [39686].
See #39227.
Fixes #40405.

Merges[40405] to the 4.7 branch.

Note: See TracTickets for help on using tickets.