diff --git a/editor/components/post-preview-button/index.js b/editor/components/post-preview-button/index.js
index f9037ae64f810..034dcefb643fe 100644
--- a/editor/components/post-preview-button/index.js
+++ b/editor/components/post-preview-button/index.js
@@ -16,7 +16,7 @@ export class PostPreviewButton extends Component {
constructor() {
super( ...arguments );
- this.saveForPreview = this.saveForPreview.bind( this );
+ this.openPreviewWindow = this.openPreviewWindow.bind( this );
}
componentDidUpdate( prevProps ) {
@@ -25,7 +25,7 @@ export class PostPreviewButton extends Component {
// This relies on the window being responsible to unset itself when
// navigation occurs or a new preview window is opened, to avoid
// unintentional forceful redirects.
- if ( previewLink && previewLink !== prevProps.previewLink ) {
+ if ( previewLink && ! prevProps.previewLink ) {
this.setPreviewWindowLink( previewLink );
}
}
@@ -38,11 +38,14 @@ export class PostPreviewButton extends Component {
*/
setPreviewWindowLink( url ) {
const { previewWindow } = this;
- if ( ! previewWindow || previewWindow.location.href === url ) {
- return;
- }
- previewWindow.location = url;
+ // Once popup redirect is evaluated, even if already closed, delete
+ // reference to avoid later assignment of location in a post update.
+ delete this.previewWindow;
+
+ if ( previewWindow && ! previewWindow.closed ) {
+ previewWindow.location = url;
+ }
}
getWindowTarget() {
@@ -50,28 +53,41 @@ export class PostPreviewButton extends Component {
return `wp-preview-${ postId }`;
}
- saveForPreview( event ) {
- const { isDirty, isNew } = this.props;
+ /**
+ * Handles a click event to open a popup window and prevent default click
+ * behavior if the post is either autosaveable or has a previously assigned
+ * preview link to be shown in the popup window target. Triggers autosave
+ * if post is autosaveable.
+ *
+ * @param {MouseEvent} event Click event from preview button click.
+ */
+ openPreviewWindow( event ) {
+ const { isAutosaveable, previewLink } = this.props;
- // Let default link behavior occur if no changes to saved post
- if ( ! isDirty && ! isNew ) {
+ // If there are no changes to autosave, we cannot perform the save, but
+ // if there is an existing preview link (e.g. previous published post
+ // autosave), it should be reused as the popup destination.
+ if ( ! isAutosaveable && ! previewLink ) {
return;
}
- // Save post prior to opening window
- this.props.autosave();
-
// Open a popup, BUT: Set it to a blank page until save completes. This
// is necessary because popups can only be opened in response to user
// interaction (click), but we must still wait for the post to save.
event.preventDefault();
this.previewWindow = window.open(
- 'about:blank',
+ isAutosaveable ? 'about:blank' : previewLink,
this.getWindowTarget()
);
+ if ( ! isAutosaveable ) {
+ return;
+ }
+
+ this.props.autosave();
+
const markup = `
-
+
Please wait…
Generating preview.
@@ -79,7 +95,7 @@ export class PostPreviewButton extends Component {
body {
margin: 0;
}
- div {
+ .editor-post-preview-button__interstitial-message {
display: flex;
flex-direction: column;
align-items: center;
@@ -95,10 +111,6 @@ export class PostPreviewButton extends Component {
this.previewWindow.document.write( markup );
this.previewWindow.document.close();
-
- // When popup is closed or redirected by setPreviewWindowLink, delete
- // reference to avoid later assignment of location in a post update.
- this.previewWindow.onbeforeunload = () => delete this.previewWindow;
}
render() {
@@ -109,7 +121,7 @@ export class PostPreviewButton extends Component {
className="editor-post-preview"
isLarge
href={ currentPostLink }
- onClick={ this.saveForPreview }
+ onClick={ this.openPreviewWindow }
target={ this.getWindowTarget() }
disabled={ ! isSaveable }
>
@@ -132,6 +144,7 @@ export default compose( [
isEditedPostDirty,
isEditedPostNew,
isEditedPostSaveable,
+ isEditedPostAutosaveable,
} = select( 'core/editor' );
const {
getPostType,
@@ -144,6 +157,7 @@ export default compose( [
isDirty: isEditedPostDirty(),
isNew: isEditedPostNew(),
isSaveable: isEditedPostSaveable(),
+ isAutosaveable: isEditedPostAutosaveable(),
isViewable: get( postType, [ 'viewable' ], false ),
};
} ),
diff --git a/editor/components/post-preview-button/test/index.js b/editor/components/post-preview-button/test/index.js
index 9baad77fc1f1a..899d7f20a459e 100644
--- a/editor/components/post-preview-button/test/index.js
+++ b/editor/components/post-preview-button/test/index.js
@@ -20,26 +20,6 @@ describe( 'PostPreviewButton', () => {
expect( setter ).not.toHaveBeenCalled();
} );
- it( 'should do nothing if the preview window is already at url location', () => {
- const url = 'https://wordpress.org';
- const setter = jest.fn();
- const wrapper = shallow(
);
- wrapper.instance().previewWindow = {
- get location() {
- return {
- href: url,
- };
- },
- set location( value ) {
- setter( value );
- },
- };
-
- wrapper.instance().setPreviewWindowLink( url );
-
- expect( setter ).not.toHaveBeenCalled();
- } );
-
it( 'set preview window location to url', () => {
const url = 'https://wordpress.org';
const setter = jest.fn();
@@ -80,18 +60,19 @@ describe( 'PostPreviewButton', () => {
isSaveable
modified="2017-08-03T15:05:50" />
);
- wrapper.instance().previewWindow = { location: {} };
+
+ const previewWindow = { location: {} };
+
+ wrapper.instance().previewWindow = previewWindow;
wrapper.setProps( { previewLink: 'https://wordpress.org/?p=1' } );
- expect(
- wrapper.instance().previewWindow.location
- ).toBe( 'https://wordpress.org/?p=1' );
+ expect( previewWindow.location ).toBe( 'https://wordpress.org/?p=1' );
} );
} );
- describe( 'saveForPreview()', () => {
- function assertForSave( props, isExpectingSave ) {
+ describe( 'openPreviewWindow()', () => {
+ function assertForPreview( props, expectedPreviewURL, isExpectingSave ) {
const autosave = jest.fn();
const preventDefault = jest.fn();
const windowOpen = window.open;
@@ -105,50 +86,59 @@ describe( 'PostPreviewButton', () => {
} );
const wrapper = shallow(
-
+
);
wrapper.simulate( 'click', { preventDefault } );
- if ( isExpectingSave ) {
- expect( autosave ).toHaveBeenCalled();
+ if ( expectedPreviewURL ) {
expect( preventDefault ).toHaveBeenCalled();
- expect( window.open ).toHaveBeenCalled();
- expect( wrapper.instance().previewWindow.document.write ).toHaveBeenCalled();
+ expect( window.open ).toHaveBeenCalledWith( expectedPreviewURL, 'wp-preview-1' );
} else {
- expect( autosave ).not.toHaveBeenCalled();
expect( preventDefault ).not.toHaveBeenCalled();
expect( window.open ).not.toHaveBeenCalled();
}
window.open = windowOpen;
+
+ expect( autosave.mock.calls ).toHaveLength( isExpectingSave ? 1 : 0 );
+ if ( isExpectingSave ) {
+ expect( wrapper.instance().previewWindow.document.write ).toHaveBeenCalled();
+ }
+
+ return wrapper;
}
- it( 'should do nothing if not dirty for saved post', () => {
- assertForSave( {
- postId: 1,
- isNew: false,
- isDirty: false,
- isSaveable: true,
- }, false );
+ it( 'should do nothing if neither autosaveable nor preview link available', () => {
+ assertForPreview( {
+ isAutosaveable: false,
+ previewLink: undefined,
+ }, null, false );
} );
- it( 'should save if not dirty for new post', () => {
- assertForSave( {
- postId: 1,
- isNew: true,
- isDirty: false,
- isSaveable: true,
- }, true );
+ it( 'should save for autosaveable post with preview link', () => {
+ assertForPreview( {
+ isAutosaveable: true,
+ previewLink: 'https://wordpress.org/?p=1&preview=true',
+ }, 'about:blank', true );
} );
- it( 'should open a popup window', () => {
- assertForSave( {
- postId: 1,
- isNew: true,
- isDirty: true,
- isSaveable: true,
- }, true );
+ it( 'should save for autosaveable post without preview link', () => {
+ assertForPreview( {
+ isAutosaveable: true,
+ previewLink: undefined,
+ }, 'about:blank', true );
+ } );
+
+ it( 'should not save but open a popup window if not autosaveable but preview link available', () => {
+ assertForPreview( {
+ isAutosaveable: false,
+ previewLink: 'https://wordpress.org/?p=1&preview=true',
+ }, 'https://wordpress.org/?p=1&preview=true', false );
} );
} );
diff --git a/test/e2e/specs/preview.test.js b/test/e2e/specs/preview.test.js
index 76f92ca706965..5e56516a3e38b 100644
--- a/test/e2e/specs/preview.test.js
+++ b/test/e2e/specs/preview.test.js
@@ -23,8 +23,85 @@ describe( 'Preview', () => {
await newPost();
} );
+ let lastPreviewPage;
+
+ /**
+ * Clicks the preview button and returns the generated preview window page,
+ * either the newly created tab or the redirected existing target. This is
+ * required because Chromium infuriatingly disregards same target name in
+ * specific undetermined circumstances, else our efforts to reuse the same
+ * popup have been fruitless and exhausting. It is worth exploring further,
+ * perhaps considering factors such as origin of the interstitial page (the
+ * about:blank placeholder screen), or whether the preview link default
+ * behavior is used / prevented by the display of the popup window of the
+ * same target name. Resolves only once the preview page has finished
+ * loading.
+ *
+ * @return {Promise} Promise resolving with focused, loaded preview page.
+ */
+ async function getOpenedPreviewPage() {
+ const eventHandlers = [];
+
+ page.click( '.editor-post-preview' );
+
+ const race = [
+ new Promise( ( resolve ) => {
+ async function onBrowserTabOpened( target ) {
+ const targetPage = await target.page();
+ resolve( targetPage );
+ }
+ browser.once( 'targetcreated', onBrowserTabOpened );
+ eventHandlers.push( [ browser, 'targetcreated', onBrowserTabOpened ] );
+ } ),
+ ];
+
+ if ( lastPreviewPage ) {
+ race.push( new Promise( async ( resolve ) => {
+ function onLastPreviewPageLoaded() {
+ resolve( lastPreviewPage );
+ }
+
+ lastPreviewPage.once( 'load', onLastPreviewPageLoaded );
+ eventHandlers.push( [ lastPreviewPage, 'load', onLastPreviewPageLoaded ] );
+ } ) );
+ }
+
+ // The preview page is whichever of the two resolves first:
+ // - A new tab has opened.
+ // - An existing tab is reused and navigates.
+ const previewPage = await Promise.race( race );
+
+ // Since there may be lingering event handlers from whichever of the
+ // race candidates had lost, remove all handlers.
+ eventHandlers.forEach( ( [ target, event, handler ] ) => {
+ target.removeListener( event, handler );
+ } );
+
+ // If a new preview tab is opened and there was a previous one, close
+ // the previous tab.
+ if ( lastPreviewPage && lastPreviewPage !== previewPage ) {
+ await lastPreviewPage.close();
+ }
+
+ lastPreviewPage = previewPage;
+
+ // Allow preview to generate if interstitial is visible.
+ const isGeneratingPreview = await previewPage.evaluate( () => (
+ !! document.querySelector( '.editor-post-preview-button__interstitial-message' )
+ ) );
+
+ if ( isGeneratingPreview ) {
+ await previewPage.waitForNavigation();
+ }
+
+ await previewPage.bringToFront();
+
+ return previewPage;
+ }
+
it( 'Should open a preview window for a new post', async () => {
const editorPage = page;
+ let previewPage;
// Disabled until content present.
const isPreviewDisabled = await page.$$eval(
@@ -35,26 +112,7 @@ describe( 'Preview', () => {
await editorPage.type( '.editor-post-title__input', 'Hello World' );
- // Don't proceed with autosave until preview window page is resolved.
- await editorPage.setRequestInterception( true );
-
- let [ , previewPage ] = await Promise.all( [
- editorPage.click( '.editor-post-preview' ),
- new Promise( ( resolve ) => {
- browser.once( 'targetcreated', async ( target ) => {
- resolve( await target.page() );
- } );
- } ),
- ] );
-
- // Interstitial screen while save in progress.
- expect( previewPage.url() ).toBe( 'about:blank' );
-
- // Release request intercept should allow redirect to occur after save.
- await Promise.all( [
- previewPage.waitForNavigation(),
- editorPage.setRequestInterception( false ),
- ] );
+ previewPage = await getOpenedPreviewPage();
// When autosave completes for a new post, the URL of the editor should
// update to include the ID. Use this to assert on preview URL.
@@ -72,40 +130,18 @@ describe( 'Preview', () => {
// Return to editor to change title.
await editorPage.bringToFront();
await editorPage.type( '.editor-post-title__input', '!' );
-
- // Second preview should reuse same popup frame, with interstitial.
- await editorPage.setRequestInterception( true );
- await Promise.all( [
- editorPage.click( '.editor-post-preview' ),
- // Note: `load` event is used since, while a `window.open` with
- // `about:blank` is called, the target window doesn't actually
- // navigate to `about:blank` (it is treated as noop). But when
- // the `document.write` + `document.close` of the interstitial
- // finishes, a `load` event is fired.
- new Promise( ( resolve ) => previewPage.once( 'load', resolve ) ),
- ] );
- await editorPage.setRequestInterception( false );
-
- // Wait for preview to load.
- await new Promise( ( resolve ) => {
- previewPage.once( 'load', resolve );
- } );
+ previewPage = await getOpenedPreviewPage();
// Title in preview should match updated input.
previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );
expect( previewTitle ).toBe( 'Hello World!' );
// Pressing preview without changes should bring same preview window to
- // front and reload, but should not show interstitial. Intercept editor
- // requests in case a save attempt occurs, to avoid race condition on
- // the load event and title retrieval.
+ // front and reload, but should not show interstitial.
await editorPage.bringToFront();
- await editorPage.setRequestInterception( true );
- await editorPage.click( '.editor-post-preview' );
- await new Promise( ( resolve ) => previewPage.once( 'load', resolve ) );
+ previewPage = await getOpenedPreviewPage();
previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );
expect( previewTitle ).toBe( 'Hello World!' );
- await editorPage.setRequestInterception( false );
// Preview for published post (no unsaved changes) directs to canonical
// URL for post.
@@ -116,35 +152,15 @@ describe( 'Preview', () => {
page.click( '.editor-post-publish-panel__header button' ),
] );
expectedPreviewURL = await editorPage.$eval( '.notice-success a', ( node ) => node.href );
- // Note / Temporary: It's expected that Chrome should reuse the same
- // tab with window name `wp-preview-##`, yet in this instance a new tab
- // is unfortunately created.
- previewPage = ( await Promise.all( [
- editorPage.click( '.editor-post-preview' ),
- new Promise( ( resolve ) => {
- browser.once( 'targetcreated', async ( target ) => {
- resolve( await target.page() );
- } );
- } ),
- ] ) )[ 1 ];
+ previewPage = await getOpenedPreviewPage();
expect( previewPage.url() ).toBe( expectedPreviewURL );
// Return to editor to change title.
await editorPage.bringToFront();
await editorPage.type( '.editor-post-title__input', ' And more.' );
- // Published preview should reuse same popup frame, with interstitial.
- await editorPage.setRequestInterception( true );
- await Promise.all( [
- editorPage.click( '.editor-post-preview' ),
- new Promise( ( resolve ) => previewPage.once( 'load', resolve ) ),
- ] );
- await editorPage.setRequestInterception( false );
-
- // Wait for preview to load.
- await new Promise( ( resolve ) => {
- previewPage.once( 'load', resolve );
- } );
+ // Published preview should reuse same popup frame.
+ previewPage = await getOpenedPreviewPage();
// Title in preview should match updated input.
previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );
@@ -154,5 +170,18 @@ describe( 'Preview', () => {
const { query } = parse( previewPage.url(), true );
expect( query ).toHaveProperty( 'preview_id' );
expect( query ).toHaveProperty( 'preview_nonce' );
+
+ // Return to editor. Previewing already-autosaved preview tab should
+ // reuse the opened tab, skipping interstitial. This resolves an edge
+ // cases where the post is dirty but not autosaveable (because the
+ // autosave is already up-to-date).
+ //
+ // See: https://github.com/WordPress/gutenberg/issues/7561
+ await editorPage.bringToFront();
+ previewPage = await getOpenedPreviewPage();
+
+ // Title in preview should match updated input.
+ previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );
+ expect( previewTitle ).toBe( 'Hello World! And more.' );
} );
} );