#20337 closed defect (bug) (fixed)

On theme customizer, back button should close preview mode

Reported by: jane Owned by: koopersmith
Priority: normal Milestone: 3.4
Component: Administration Version:
Severity: normal Keywords: has-patch
Cc:

Description

From #19910:

Should browser back button close Theme Customizer rather than return to a previous page?

It should return to Install/Manage Themes.

Attachments (3)

customize-loader.dev.js.patch (892 bytes) - added by benkulbertis 14 months ago.
20337.patch (910 bytes) - added by SergeyBiryukov 14 months ago.
20337.2.patch (2.0 KB) - added by SergeyBiryukov 13 months ago.

Download all attachments as: .zip

Change History (11)

I made an attempt at this using hashes, however I don't quite have it. It works as expected the first run through, but after clicking the back button and having the overlay close something is not reset correctly because the "Customize" link no longer functions. Maybe someone can build from the code I wrote thus far.

  • Keywords has-patch added; needs-patch removed

20337.patch builds on Ben's patch. Seems to work fine in my testing.

comment:3 follow-up: ↓ 4   koopersmith14 months ago

I haven't had a chance to test these patches yet, but I'd rather we use the window.history API.

comment:4 in reply to: ↑ 3   azaozz13 months ago

Replying to koopersmith:

+1 for using window.history if possible or pop push state.

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

comment:5 follow-up: ↓ 6   SergeyBiryukov13 months ago

Not sure how to use windows.history in this context, since it doesn't allow changes to history.

20337.2.patch is another attempt. After clicking browser's Back button, Forward button works as well (returns to Theme Customizer).

Tested in Firefox 11, Chrome 18, IE 8, Opera 11.62, Safari 5.1.5.

The patch uses onhashchange rather than onpopstate, since the latter is not supported by IE 8.

The if ( Loader.iframe ) check in line 68 is due to Loader.close() being called twice when clicking .close-full-overlay:

  1. Directly in this.element.click.
  2. Via window.onhashchange.

It would probably be a bit cleaner to remove the call from this.element.click and let onhashchange handle the closing.

comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7   nacin13 months ago

Replying to SergeyBiryukov:

Not sure how to use windows.history in this context, since it doesn't allow changes to history.

I think window.history was a reference to history.pushState().

comment:7 in reply to: ↑ 6   SergeyBiryukov13 months ago

Replying to nacin:

I think window.history was a reference to history.pushState().

Ah, thanks. I thought of pushState(), but was confused by the fact that it's not used anywhere in core yet. Seems replaceState() was once added in [16615], but then removed in [18028], along with list-table.dev.js.

Wouldn't using pushState() introduce an inconsistent behaviour between browsers, since it's not supported by IE 9? Or should we bring in some external library or another workaround as a fallback?

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

In [20488]:

Theme Customizer: Integrate with browser history. Use window.history by default, with window.onhashchange as a fallback. fixes #20337, see #19910.

Note: See TracTickets for help on using tickets.