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.' ); } ); } );