Fix HTML drop issues with the Windows browsers #37151
Conversation
The File drops intercept HTML drops, and it takes precedence with the recent Windows chrome version. Changing the order of the checks fixes the problem.
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @sathyapulse! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
mcsf
left a comment
There was a problem hiding this comment.
I don’t mind switching the order of the conditions, but it’s important to understand what has changed. If getFilesFromDataTransfer isn’t coming back empty, what is it returning? Are there values needing pruning, or is our function not up to spec?
The function The DropZone has been rewritten without a provider recently here, and it changed the code significantly. I'm not sure if that's a regression from the rewrite. |
Do you mean that, when the dragged HTML object contains an image, newer versions of browsers are being smart about it and attaching a File data-transfer item alongside the HTML item? If so, then the fix makes sense. |
ellatrix
left a comment
There was a problem hiding this comment.
Sounds good. I do vaguely remember Windows attaching an image version of copied text or html (which is a bit weird), so reordering makes sense.
| if ( files.length ) { | ||
| _onFilesDrop( files ); | ||
| } else if ( html ) { | ||
| if ( html ) { |
There was a problem hiding this comment.
Could you just add a comment here about why the order is important and what this extra image in Windows is?
There was a problem hiding this comment.
I guess that would be good if a manual test reveals a problem.
There was a problem hiding this comment.
@ellatrix: should the two handlers be only defined once?
There was a problem hiding this comment.
@sathyapulse: I agree with Ella. Once you have fixed the order in the other handler, and added comments to each to justify the issue (possibly linking to this PR), let us know and we can merge this.
@mcsf Yes, the newer browser version on Windows treats it as a file blob object and attaches/uploads the file to the media library and adds an image block. I did some tests to confirm the browser implementation. On Mac Chrome 96, the file object is empty, and WP adds the HTML with an external link. The file object is empty when the chrome browser is downgraded to version 95 on Windows. CC: @ellatrix |
| if ( files.length ) { | ||
| _onFilesDrop( files ); | ||
| } else if ( html ) { | ||
| if ( html ) { |
There was a problem hiding this comment.
@sathyapulse: I agree with Ella. Once you have fixed the order in the other handler, and added comments to each to justify the issue (possibly linking to this PR), let us know and we can merge this.
Add inline docs to explain the changes.
Description
The File drops intercept HTML drops, and it takes precedence with the recent Windows chrome version. It causes the plugins relying on the HTML drop to stop working. Changing the order of the checks fixes the problem — Fixes #36824.
How has this been tested?
The steps to test has been documented in issue #36824.
Screenshots
Types of changes
Changing the order of the checks fixes the problem.
Checklist:
*.native.jsfiles for terms that need renaming or removal).