diff --git a/packages/editor/src/components/post-preview-button/index.js b/packages/editor/src/components/post-preview-button/index.js index d35b29a699eb26..9190a713b3e79d 100644 --- a/packages/editor/src/components/post-preview-button/index.js +++ b/packages/editor/src/components/post-preview-button/index.js @@ -28,22 +28,22 @@ export class PostPreviewButton extends Component { // unintentional forceful redirects. if ( previewLink && ! prevProps.previewLink ) { this.setPreviewWindowLink( previewLink ); + + // Once popup redirect is evaluated, even if already closed, delete + // reference to avoid later assignment of location in post update. + delete this.previewWindow; } } /** * Sets the preview window's location to the given URL, if a preview window - * exists and is not already at that location. + * exists and is not closed. * * @param {string} url URL to assign as preview window location. */ setPreviewWindowLink( url ) { const { previewWindow } = this; - // 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; } @@ -55,37 +55,29 @@ export class PostPreviewButton extends Component { } /** - * 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. + * Opens a popup window, navigating user to a preview of the current post. + * Triggers autosave if post is autosaveable. */ - openPreviewWindow( event ) { + openPreviewWindow() { const { isAutosaveable, previewLink, currentPostLink } = this.props; + // 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. + if ( ! this.previewWindow ) { + this.previewWindow = window.open( '', this.getWindowTarget() ); + } + // 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 && currentPostLink ) { - this.previewWindow = window.open( - currentPostLink, - this.getWindowTarget() - ); + this.setPreviewWindowLink( currentPostLink ); return; } - // 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( - isAutosaveable ? 'about:blank' : previewLink, - this.getWindowTarget() - ); - if ( ! isAutosaveable ) { + this.setPreviewWindowLink( previewLink ); return; } diff --git a/packages/editor/src/components/post-preview-button/test/index.js b/packages/editor/src/components/post-preview-button/test/index.js index 7084654785317e..81c353f83dbf4f 100644 --- a/packages/editor/src/components/post-preview-button/test/index.js +++ b/packages/editor/src/components/post-preview-button/test/index.js @@ -76,10 +76,13 @@ describe( 'PostPreviewButton', () => { describe( 'openPreviewWindow()', () => { function assertForPreview( props, expectedPreviewURL, isExpectingSave ) { const autosave = jest.fn(); - const preventDefault = jest.fn(); + const setLocation = jest.fn(); const windowOpen = window.open; window.open = jest.fn( () => { return { + set location( url ) { + setLocation( url ); + }, document: { write: jest.fn(), close: jest.fn(), @@ -95,17 +98,14 @@ describe( 'PostPreviewButton', () => { /> ); - wrapper.simulate( 'click', { preventDefault } ); + wrapper.simulate( 'click' ); - if ( expectedPreviewURL ) { - if ( expectedPreviewURL !== props.currentPostLink ) { - expect( preventDefault ).toHaveBeenCalled(); - } + expect( window.open ).toHaveBeenCalledWith( '', 'wp-preview-1' ); - expect( window.open ).toHaveBeenCalledWith( expectedPreviewURL, 'wp-preview-1' ); + if ( expectedPreviewURL ) { + expect( setLocation ).toHaveBeenCalledWith( expectedPreviewURL ); } else { - expect( preventDefault ).not.toHaveBeenCalled(); - expect( window.open ).not.toHaveBeenCalled(); + expect( setLocation ).not.toHaveBeenCalled(); } window.open = windowOpen; @@ -131,14 +131,14 @@ describe( 'PostPreviewButton', () => { assertForPreview( { isAutosaveable: true, previewLink: 'https://wordpress.org/?p=1&preview=true', - }, 'about:blank', true ); + }, null, true ); } ); it( 'should save for autosaveable post without preview link', () => { assertForPreview( { isAutosaveable: true, previewLink: undefined, - }, 'about:blank', true ); + }, null, true ); } ); it( 'should not save but open a popup window if not autosaveable but preview link available', () => { diff --git a/test/e2e/specs/preview.test.js b/test/e2e/specs/preview.test.js index 5e56516a3e38b2..5e3fb34c2c67b5 100644 --- a/test/e2e/specs/preview.test.js +++ b/test/e2e/specs/preview.test.js @@ -23,85 +23,23 @@ 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. + * Given a Puppeteer Page instance for a preview window, clicks Preview and + * awaits the window navigation. + * + * @param {puppeteer.Page} previewPage Page on which to await navigation. * - * @return {Promise} Promise resolving with focused, loaded preview page. + * @return {Promise} Promise resolving once navigation completes. */ - 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; + function waitForPreviewNavigation( previewPage ) { + return Promise.all( [ + previewPage.waitForNavigation(), + page.click( '.editor-post-preview' ), + ] ); } 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( @@ -112,7 +50,15 @@ describe( 'Preview', () => { await editorPage.type( '.editor-post-title__input', 'Hello World' ); - previewPage = await getOpenedPreviewPage(); + await page.click( '.editor-post-preview' ); + + const previewPage = await new Promise( ( resolve ) => { + async function onBrowserTabOpened( target ) { + const targetPage = await target.page(); + resolve( targetPage ); + } + browser.once( 'targetcreated', onBrowserTabOpened ); + } ); // 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. @@ -130,7 +76,7 @@ describe( 'Preview', () => { // Return to editor to change title. await editorPage.bringToFront(); await editorPage.type( '.editor-post-title__input', '!' ); - previewPage = await getOpenedPreviewPage(); + await waitForPreviewNavigation( previewPage ); // Title in preview should match updated input. previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent ); @@ -139,7 +85,7 @@ describe( 'Preview', () => { // Pressing preview without changes should bring same preview window to // front and reload, but should not show interstitial. await editorPage.bringToFront(); - previewPage = await getOpenedPreviewPage(); + await waitForPreviewNavigation( previewPage ); previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent ); expect( previewTitle ).toBe( 'Hello World!' ); @@ -152,15 +98,13 @@ describe( 'Preview', () => { page.click( '.editor-post-publish-panel__header button' ), ] ); expectedPreviewURL = await editorPage.$eval( '.notice-success a', ( node ) => node.href ); - previewPage = await getOpenedPreviewPage(); + await waitForPreviewNavigation( previewPage ); 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. - previewPage = await getOpenedPreviewPage(); + await waitForPreviewNavigation( previewPage ); // Title in preview should match updated input. previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent ); @@ -178,7 +122,7 @@ describe( 'Preview', () => { // // See: https://github.com/WordPress/gutenberg/issues/7561 await editorPage.bringToFront(); - previewPage = await getOpenedPreviewPage(); + await waitForPreviewNavigation( previewPage ); // Title in preview should match updated input. previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );