3.4 Preview/Customize page "Return to Manage Themes" link doesn't work as expected

[TomAuger]

The link that's supposed to take you back to the Manage Themes page doesn't work as expected (browser: Google Chrome).

If you are customizing a theme and make any changes, the "Return" link at the top of the sidebar doesn't take you back to the Manage Themes page.

For each change that you make (ie: when editing one of the fields), you have to click that many times on the Return link before you get taken back to the main page. It's as if each "autosave" onChange of each input field gets stored in the browser's history and you have to go back that many times before you go back to the page you're expecting.

And it doesn't work like an incremental Undo either - clicking the link even once undoes all the changes you made (unless you Save). But you still have to click the link that many times before it actually takes you back.

While I'm on the topic - I wonder whether it's a mistake to make the "Save" button take you out of the Customize mode? As an end user, it surprised me that it didn't just save, but leave me there.

[SergeyBiryukov]

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

[helenyhou]

Related: #20535

[koopersmith]

Thanks, Helen. To fix this, we should declare the iframe's src attribute in the html when we create the iframe (instead of setting it afterwards).

[koopersmith]

Replying to koopersmith:

To fix this, we should declare the iframe's src attribute in the html when we create the iframe (instead of setting it afterwards).

After looking through the code again, this isn't a possible solution because we need to send the control data to the iframe via POST (which is currently being accomplished by setting the URL as the action of the form and the iframe as the target).

Instead, I'm looking into whipping up a ajax-based solution that will inject the proper html into the iframe.

[nacin]

Per koopersmith, this should be considered a blocker for beta 4.

[TomAuger]

Guys, as a workaround until you implement the ajax-based solution, would you consider using JavaScript and the browser history to go back to the correct page?

[koopersmith]

Replying to TomAuger:

Guys, as a workaround until you implement the ajax-based solution, would you consider using JavaScript and the browser history to go back to the correct page?

I don't think this is possible; if the browser records state as a full refresh, history.go( -1 * numberOfChanges ) will, in turn, trigger a full refresh. Plus, this could cause additional edge cases in non-history compliant browsers.

I'll land the ajax fix shortly.

[koopersmith]

In [20645]:

Theme Customizer: Migrate to an ajax-based solution for refreshing the preview and saving. see #20507, #19910.

• Use ajax-based saving, add saving indicator.
• Use ajax-based refreshing instead of form targets.
• Instead of using hidden inputs with prefixed names to track the canonical data, use the values stored in wp.customize. Encode the values as JSON before sending to avoid bugs with ids that contain square brackets (PHP mangles POST values with nested brackets).
• Use wp.customize.Previewer solely for the purpose of previewing; move the postMessage connection with the parent frame and other unrelated code snippets into the 'ready' handler.

[koopersmith]

Please test the changes in [20645] across browsers — I'd like to identify and fix any cross-browser issues as quickly as possible. I'm leaving it open for this reason.

This ticket is no longer a blocker for beta 4. Go team.

[ryan]

XMLHttpRequest cannot load http://foo.wordpress.org/. Origin https://foo.wordpress.org is not allowed by Access-Control-Allow-Origin.

We need to figure out origin access, especially for multisite installs that use a domain mapping plugin. Thoughts?

[ryan]

Tentative plan after talking to koopersmith and nacin:

• Serve both the customizer and the preview from the front so they both have the same origin.
• If the admin is forced SSL, serve the customizer and preview over SSL as well.
• Save via ajax when the origin and the destination match, as is currently done.
• Save via POST and then redirect back to customizer when the origin and dest do not match. This avoids origin issues and allows referencing the admin auth cookies rather than just the logged in cookie.

[koopersmith]

Attached patch successfully serves the customizer and preview from the front end.

Forced SSL trickery and save via POST are *not* included in this patch.

[ryan]

Handle $wp_scripts not being set

[ryan]

PHP Notice:  Trying to get property of non-object in /wp-includes/customize-controls.php on line 17

Evidently my test sites don't call any of the script loader functions that setup $wp_scripts before wp_loaded is fire. Patch handles this.

[ryan]

Allow origin

[mdawaffe]

Since HTTP and HTTPS aren't the same origin, we also need to explicitly send credentials in the AJAX request and allow credentials in the CORS response.

Attached in 20507.4.diff

Also note that in ​http://www.w3.org/TR/cors/#resource-requests, the spec states we should not set any Access-Control headers in the response if the Origin in the request does not match our list of acceptable origins. I think the @header() calls should be moved into the if block, and the else block should be removed. Not patched.

[koopersmith]

In [20741]:

Theme Customizer: Add cross-domain handling for when the admin and front-end are different origins. Handles both ajax and postMessage calls. props rboren, mdawaffe, nacin. see #20507, #19910.

[ryan]

Origin functions

[ryan]

Just filters for now.

[ryan]

With a send origin headers func.

[ryan]

Cleanup, phpdoc

[ryan]

In [20793]:

Don't force frontend scheme to match backend. This fails if the frontend doesn't have proper SSL certs. Access-Control-Allow-Origin handles this without the need to make the schemes match. see #20507

[johnbillion]

This is still broken. Steps to reproduce:

1. Visit the theme customizer and change some settings if you feel like it.
2. Click a link to another page on the site.
3. Click 'Return to Manage Themes'.

You'll simply be taken back a page in your browser's history instead of being taken back to the Themes screen.

[ocean90]

Which browser? Couldn't reproduce it with Chrome dev and Firefox 12 on OS X.

[johnbillion]

Ah-ha, this is happening when you click a link which results in a server side redirect. An example of this is clicking the Random Member or Random Group link in the BuddyPress admin bar menu. After the redirect has happened, the theme customizer controls also stop working.

[johnbillion]

Nope, even more specifically, it happens after you've clicked another link after you've clicked a link which results in a server side redirect.

[nacin]

We should be able to block/override Location headers as well as hook into wp_redirect() and short-circuit. (Might not prevent the "exit" after one though.)

[nacin]

Below is a test case (drop it into mu-plugins) so it can be consistently reproduced.

It sounds like our best bet is to watch for and intercept a 30x in JS. Then check where the redirect is going. If it is going somewhere internally (based on our origins), POST to that URL. If it is going somewhere externally, send us back to the home page with a POST.

It looks like our current check that prevents external links from working also block URLs that are relative, a hash, or a query string. All three of these are allowed in esc_url() and should be considered internal URLs.

add_action( 'get_header', function() {
	echo '<a href="' . esc_url( add_query_arg( 'redirect', 'internal' ) ) . '">Redirect internally</a> &bull; <a href="' . esc_url( add_query_arg( 'redirect', 'external' ) )  . '">Redirect externally</a><br />';
	echo '<a href="/">Root-relative link to home page that gets blocked :-(</a><br />';
	echo '<a href="#footer">Hash link that gets blocked :-(</a><br />';
	echo '<a href="?redirect=internal">Query string link that gets blocked :-(</a>';
} );

add_action( 'template_redirect', function() {
	if ( ! isset( $_GET['redirect'] ) )
		return;

	if ( 'internal' == $_GET['redirect'] )
		wp_redirect( get_permalink( 1 ) );
	else
		wp_redirect( 'http://wordpress.org/' );

	exit;
} );

[nacin]

Per ​http://stackoverflow.com/a/2573589, it is not possible to detect a redirect.

What koopersmith is now working on, is forcing wp_redirect() to return a 200 along with a Location header, then looking for a Location header, testing the target URL, and then either doing a POST (internal or killing the link (external).

There is still going to be an issue with manual header('Location:')'s. We can also hook into shutdown and force a 200 there, but it might be after headers get sent. I suggested we insert some hidden markup into the iframe that can be looked for to ensure the page we got back was generated by the customizer. And if not, we know something went wrong and we can send them to the home page.

Other things we may want to consider: Disabling canonical, cron, and update checks (especially since active theme reporting will be inaccurate).

[koopersmith]

In [20861]:

Theme Customizer: Properly handle redirects in the preview by setting wp_redirect_status to 200. props nacin, see #20507, #19910.

[koopersmith]

In [20863]:

Theme Customizer: Add a base element to the preview's head element to allow relative links (root, hash, and query strings). see #20507, #19910.

[jane]

The Return to Manage Themes link has been removed, in lieu of Save & Activate (or Save & Publish) button returning to the Manage Themes screen after submit, and Cancel button returning to the Manage Themes screen.

[koopersmith]

In [20882]:

Theme Customizer: Improve accuracy of identifying internal urls. see #20507, #19910.

The 'customize_preview_link' filter has been replaced by 'customize_allowed_urls'.
Improved accuracy when checking for wp-admin.
Improved accuracy when attempting to match the schemes of the control and preview frames.
Improved accuracy of internal link whitelist.

[nacin]

Fixed? Anything left?

[nacin]

In [20924]:

Do not spawn cron or trigger update checks when running the customizer.

Both can slow down the experience.

The alternate cron will issue a redirect which creates more work for the
customizer, but not exit immediately, which means shutdown will be delayed
(see future changeset from koopersmith in #20507 where we check for a token
printed on shutdown to ensure the response came from the customizer).

The update checks could also cause bad data to be sent. In particular, the
currently active theme would be incorrect.

see #20507.

[nacin]

see future changeset from koopersmith in #20507 where we check for a token

printed on shutdown to ensure the response came from the customizer

That is the only thing left in this ticket.

[koopersmith]

In [20925]:

Theme Customizer: Add a signature to preview requests to be super-double-ultra-sure that the customizer generated the preview. Redirects can be sneaky. fixes #20507, see #19910.

[nacin]

In [20926]:

Theme Customizer: Introduce a remove_preview_signature() method that we can employ to ensure we do not think a wp_die() is a customizer-generated preview. see #20507.

[nacin]

There have been some inquiries into why we ended up handling cross-domain issues instead of serving the preview via the admin instead of the frontend, or serving the controls via the frontend instead of the admin. Indeed, one strategy was initially decided upon above — serving the controls through the frontend. That wouldn't have worked since we needed not only the admin styles, but also register_setting(), wp-admin auth cookies, and more. But the reverse — serving the preview through the admin — was a very interesting thought.

This wp-admin endpoint to serve the preview would have needed to act like it wasn't a part of the admin for everything but the URL. In theory, it sounded pretty cool, but in practice, it would have had major side effects and would have led us down a rabbit hole of constant compatibility problems. It would have needed to use WP::main() in unusual ways and leveraged the front-end template-loader as well. We would have had to trick WP_Query into thinking it was not querying in the admin. Any theme doing is_admin() {} else {}, or using admin_init, or anything else, could have potentially broke. Any plugins hooking into WP, WP_Query, the template loader, or the template could have potentially been confused.

Mind you, all of this was to avoid something so simple as this: If you are using a mapped domain, un-map it for the customizer.

And so, we agreed to handle cross-domain origin issues the best we could (and given that it works on WP.com, we did exactly that), in order to be as compatible and as flexible as possible. But emphasis would then be placed on the simple solution of simply unmapping the front-end domain when using the customizer.

Solving our cross-domain origin issues allowed us to understand these problems and open up new doors (for example, #21024), and that (plus recommending the unmapping of the domain — hey look, problem solved!) was ultimately way less of a headache than trying to wedge the preview into the admin or vice versa.

(References: An IRC conversation from ​May 7.)