Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clipboard read/write on regular elements in e2e tests #55030

Open
wants to merge 14 commits into
base: trunk
Choose a base branch
from
Open

Fix clipboard read/write on regular elements in e2e tests #55030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4db3f88
Fix clipboard read/write on normal elements
kevin940726 Oct 4, 2023
0987f26
Try timeout
kevin940726 Oct 6, 2023
7474ce6
Fix cross-browser
kevin940726 Oct 7, 2023
8941952
Fix focus selection and unstable test
kevin940726 Oct 7, 2023
14ab6d6
Rearrange test
kevin940726 Oct 8, 2023
5b98d2c
Different approach
kevin940726 Oct 9, 2023
c519666
Change to timeout
kevin940726 Oct 9, 2023
cb0992f
Fix copy and cut too
kevin940726 Oct 11, 2023
fb2836b
Add comment and cleanup the code
kevin940726 Oct 11, 2023
04d4571
Delete unused snapshot
kevin940726 Oct 12, 2023
24c472d
Revert unused change
kevin940726 Oct 12, 2023
55c091d
Remove unused page
kevin940726 Oct 12, 2023
745f8da
Fix code test
kevin940726 Nov 15, 2023
152583b
Migrate more setClipboardData usages
kevin940726 Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 159 additions & 65 deletions packages/e2e-test-utils-playwright/src/page-utils/press-keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ let clipboardDataHolder: {
* @param clipboardData.plainText
* @param clipboardData.html
*/
export function setClipboardData(
export async function setClipboardData(
this: PageUtils,
{ plainText = '', html = '' }
) {
Expand All @@ -47,64 +47,157 @@ export function setClipboardData(
'text/html': html,
'rich-text': '',
};

// Set the clipboard data for the keyboard press below.
// This is needed for the `paste` event to be fired in case of a real key press.
await this.page.evaluate( ( data ) => {
const activeElement =
document.activeElement instanceof HTMLIFrameElement
? document.activeElement.contentDocument!.activeElement
: document.activeElement;
activeElement?.addEventListener(
'copy',
( event ) => {
event.preventDefault();
event.stopImmediatePropagation();
Object.entries( data ).forEach( ( [ type, text ] ) => {
if ( text ) {
( event as ClipboardEvent ).clipboardData?.setData(
type,
text
);
}
} );
},
{ once: true, capture: true }
);
}, clipboardDataHolder );

await this.page.keyboard.press(
isAppleOS() ? 'Meta+KeyC' : 'Control+KeyC'
);
}

async function emulateClipboard( page: Page, type: 'copy' | 'cut' | 'paste' ) {
clipboardDataHolder = await page.evaluate(
const promiseHandle = await page.evaluateHandle(
( [ _type, _clipboardData ] ) => {
const canvasDoc =
// @ts-ignore
document.activeElement?.contentDocument ?? document;
const clipboardDataTransfer = new DataTransfer();

if ( _type === 'paste' ) {
clipboardDataTransfer.setData(
'text/plain',
_clipboardData[ 'text/plain' ]
);
clipboardDataTransfer.setData(
'text/html',
_clipboardData[ 'text/html' ]
);
clipboardDataTransfer.setData(
'rich-text',
_clipboardData[ 'rich-text' ]
);
} else {
const selection = canvasDoc.defaultView.getSelection()!;
const plainText = selection.toString();
let html = plainText;
if ( selection.rangeCount ) {
const range = selection.getRangeAt( 0 );
const fragment = range.cloneContents();
html = Array.from( fragment.childNodes )
.map(
( node ) =>
( node as Element ).outerHTML ??
( node as Element ).nodeValue
)
.join( '' );
}
clipboardDataTransfer.setData( 'text/plain', plainText );
clipboardDataTransfer.setData( 'text/html', html );
}

canvasDoc.activeElement?.dispatchEvent(
new ClipboardEvent( _type, {
bubbles: true,
cancelable: true,
clipboardData: clipboardDataTransfer,
} )
);
const activeElement =
document.activeElement instanceof HTMLIFrameElement
? document.activeElement.contentDocument!.activeElement
: document.activeElement;

// Return an object with the promise handle to bypass the auto-resolving
// feature of `evaluateHandle()`.
return {
'text/plain': clipboardDataTransfer.getData( 'text/plain' ),
'text/html': clipboardDataTransfer.getData( 'text/html' ),
'rich-text': clipboardDataTransfer.getData( 'rich-text' ),
promise: new Promise< false | typeof clipboardDataHolder >(
( resolve ) => {
const timeout = setTimeout( () => {
resolve( false );
}, 50 );
Comment on lines +94 to +96
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a safeguard to resolve the Promise if the method isn't able to add an event listener, primarily due to activeElement being unavailable. Is my logic/understanding here correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is here in case the event never fires. This could happen for paste events when the element doesn't attach an event listener and it needs to fallback to native "real" event. We need a flag here that basically means hasEventHandled, and it that's false we'll fire a real key press.

I know this is all very confusing and I've been thinking about better ways to document this 😅. Maybe I should make a comparison table or something like that...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any additional documentation would be helpful here.


activeElement?.ownerDocument.addEventListener(
_type,
( event ) => {
clearTimeout( timeout );
if (
_type === 'paste' &&
! event.defaultPrevented
) {
resolve( false );
} else {
const selection =
activeElement.ownerDocument.getSelection()!;
const plainText = selection.toString();
let html = plainText;
if ( selection.rangeCount ) {
const range = selection.getRangeAt( 0 );
const fragment = range.cloneContents();
html = Array.from( fragment.childNodes )
.map(
( node ) =>
( node as Element )
.outerHTML ??
( node as Element )
.nodeValue
)
.join( '' );
}
// Get the clipboard data from the native bubbled event if it's set.
// Otherwise, compute the data from the current selection.
resolve( {
'text/plain':
event.clipboardData?.getData(
'text/plain'
) || plainText,
'text/html':
event.clipboardData?.getData(
'text/html'
) || html,
'rich-text':
event.clipboardData?.getData(
'rich-text'
) || '',
} );
}
},
{ once: true }
);

// Only dispatch the virtual events for `paste` events.
// `copy` and `cut` events are handled by the native key presses.
if ( _type === 'paste' ) {
const clipboardDataTransfer = new DataTransfer();
clipboardDataTransfer.setData(
'text/plain',
_clipboardData[ 'text/plain' ]
);
clipboardDataTransfer.setData(
'text/html',
_clipboardData[ 'text/html' ]
);
clipboardDataTransfer.setData(
'rich-text',
_clipboardData[ 'rich-text' ]
);

activeElement?.dispatchEvent(
new ClipboardEvent( _type, {
bubbles: true,
cancelable: true,
clipboardData: clipboardDataTransfer,
} )
);
}
}
),
};
},
[ type, clipboardDataHolder ] as const
);

// For `copy` and `cut` events, we first do a real key press to set the
// native clipboard data for the "real" `paste` event. Then, we listen for
// the bubbled event on the document to set the clipboard data for the
// "virtual" `paste` event. This won't work if the event handler calls
// `event.stopPropagation()`, but it's good enough for our use cases for now.
if ( type === 'copy' ) {
await page.keyboard.press( isAppleOS() ? 'Meta+KeyC' : 'Control+KeyC' );
} else if ( type === 'cut' ) {
await page.keyboard.press( isAppleOS() ? 'Meta+KeyX' : 'Control+KeyX' );
}

const clipboardData = await promiseHandle.evaluate(
( { promise } ) => promise
);
if ( clipboardData ) {
clipboardDataHolder = clipboardData;
} else if ( type === 'paste' ) {
// For `paste` events, we do the opposite: We first listen for the bubbled
// virtual event on the document and dispatch it to the active element.
// This won't work for native elements that don't have a `paste` event
// handler, so we then fallback to a real key press.
await page.keyboard.press( isAppleOS() ? 'Meta+KeyV' : 'Control+KeyV' );
}
}

const isAppleOS = () => process.platform === 'darwin';
Expand Down Expand Up @@ -152,36 +245,37 @@ export async function pressKeys(
this.page
);

let command: () => Promise< void >;
const keys = key.split( '+' ).flatMap( ( keyCode ) => {
if ( Object.prototype.hasOwnProperty.call( modifiers, keyCode ) ) {
return modifiers[ keyCode as keyof typeof modifiers ](
isAppleOS
).map( ( modifier ) =>
modifier === CTRL ? 'Control' : capitalCase( modifier )
);
} else if ( keyCode === 'Tab' && ! hasNaturalTabNavigation ) {
return [ 'Alt', 'Tab' ];
}
return keyCode;
} );
const normalizedKeys = keys.join( '+' );

let command = () => this.page.keyboard.press( normalizedKeys );

if ( key.toLowerCase() === 'primary+c' ) {
command = () => emulateClipboard( this.page, 'copy' );
} else if ( key.toLowerCase() === 'primary+x' ) {
command = () => emulateClipboard( this.page, 'cut' );
} else if ( key.toLowerCase() === 'primary+v' ) {
command = () => emulateClipboard( this.page, 'paste' );
} else {
const keys = key.split( '+' ).flatMap( ( keyCode ) => {
if ( Object.prototype.hasOwnProperty.call( modifiers, keyCode ) ) {
return modifiers[ keyCode as keyof typeof modifiers ](
isAppleOS
).map( ( modifier ) =>
modifier === CTRL ? 'Control' : capitalCase( modifier )
);
} else if ( keyCode === 'Tab' && ! hasNaturalTabNavigation ) {
return [ 'Alt', 'Tab' ];
}
return keyCode;
} );
const normalizedKeys = keys.join( '+' );
command = () => this.page.keyboard.press( normalizedKeys );
}

times = times ?? 1;
for ( let i = 0; i < times; i += 1 ) {
await command();

if ( times > 1 && pressOptions.delay ) {
// Disable reason: We explicitly want to wait for a specific amount of time.
// eslint-disable-next-line playwright/no-wait-for-timeout
await this.page.waitForTimeout( pressOptions.delay );
}
}
Expand Down
3 changes: 0 additions & 3 deletions test/e2e/specs/editor/blocks/__snapshots__/Code-should-paste-plain-text-1-chromium.txt
View file
Open in desktop

This file was deleted.

11 changes: 9 additions & 2 deletions test/e2e/specs/editor/blocks/code.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,17 @@ test.describe( 'Code', () => {
await editor.insertBlock( { name: 'core/code' } );

// Test to see if HTML and white space is kept.
pageUtils.setClipboardData( { plainText: '<img />\n\t<br>' } );
await pageUtils.setClipboardData( {
plainText: '<img />\n\t<br>',
} );

await pageUtils.pressKeys( 'primary+v' );

expect( await editor.getEditedPostContent() ).toMatchSnapshot();
await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/code',
attributes: { content: '&lt;img /><br>\t&lt;br>' },
},
] );
} );
} );
2 changes: 1 addition & 1 deletion test/e2e/specs/editor/blocks/gallery.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ test.describe( 'Gallery', () => {
} ) => {
await admin.createNewPost();

pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
plainText: `[gallery ids="${ uploadedMedia.id }"]`,
} );

Expand Down
14 changes: 7 additions & 7 deletions test/e2e/specs/editor/various/copy-cut-paste.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ test.describe( 'Copy/cut/paste', () => {
// back to default browser behaviour, allowing the browser to insert
// unfiltered HTML. When we swap out the post title in the post editor
// with the proper block, this test can be removed.
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing Unexpected await of a non-Promise (non-"Thenable") value. ESLint errors here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is possibly due to outdated build. Try running npm run dev or npm run build again and see if eslint still errors?

html: '<span style="border: 1px solid black">Hello World</span>',
} );
await pageUtils.pressKeys( 'primary+v' );
Expand All @@ -469,7 +469,7 @@ test.describe( 'Copy/cut/paste', () => {
} ) => {
await page.keyboard.type( 'ab' );
await page.keyboard.press( 'ArrowLeft' );
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
html: '<span style="border: 1px solid black">x</span>',
} );
await pageUtils.pressKeys( 'primary+v' );
Expand All @@ -487,7 +487,7 @@ test.describe( 'Copy/cut/paste', () => {
pageUtils,
editor,
} ) => {
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
html: '<pre>x</pre>',
} );
await editor.insertBlock( { name: 'core/list' } );
Expand All @@ -503,7 +503,7 @@ test.describe( 'Copy/cut/paste', () => {
attributes: { content: 'a' },
} );
await pageUtils.pressKeys( 'primary+a' );
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
plainText: 'https://wordpress.org/gutenberg',
html: '<a href="https://wordpress.org/gutenberg">https://wordpress.org/gutenberg</a>',
} );
Expand All @@ -523,7 +523,7 @@ test.describe( 'Copy/cut/paste', () => {
name: 'core/paragraph',
attributes: { content: 'a' },
} );
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
plainText: 'https://wordpress.org/gutenberg',
html: 'https://wordpress.org/gutenberg',
} );
Expand All @@ -541,7 +541,7 @@ test.describe( 'Copy/cut/paste', () => {

test( 'should embed on paste', async ( { pageUtils, editor } ) => {
await editor.insertBlock( { name: 'core/paragraph' } );
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
plainText: 'https://www.youtube.com/watch?v=FcTLMTyD2DU',
html: 'https://www.youtube.com/watch?v=FcTLMTyD2DU',
} );
Expand All @@ -562,7 +562,7 @@ test.describe( 'Copy/cut/paste', () => {
},
} );
await pageUtils.pressKeys( 'primary+a' );
pageUtils.setClipboardData( {
await pageUtils.setClipboardData( {
plainText: 'movie: b',
html: 'movie: b',
} );
Expand Down
Loading