fix: normalize DataTransfer format aliases - #1326
Open
dylanpulver wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
DataTransfer.getData(),setData()andclearData()now normalize theirformatargument the way the HTML spec requires. The format is converted to ASCII lowercase, and the shorthandstextandurlare replaced withtext/plainandtext/uri-list.Fixes #1269
Why
userEvent.paste('foo')builds itsDataTransferwithsetData('text', ...), and the stub stored that string verbatim as the item type. A paste handler readingevent.clipboardData.typestherefore saw['text'], so the check reported in #1269,types.includes('text/plain'), was false. No browser produces atexttype on a clipboard event, so handlers written against real clipboard data did not match.Two related cases were wrong in the same way on main.
setData('url', ...)stored the typeurl, which leftgetData('text/uri-list')returning an empty string. Separately,clearData('text')failed to remove an item stored astext/plain, and a format containing uppercase letters was stored and looked up as a type distinct from its lowercase spelling.The spec puts this mapping inside each of the three methods.
setData()converts the format to ASCII lowercase, then changestexttotext/plainandurltotext/uri-list, at https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-setdata.getData()repeats both steps before looking the item up, at https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-getdata, andclearData()does the same before removing one, at https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-cleardata.How
The mapping lives in a
normalizeFormat()helper insrc/utils/dataTransfer/DataTransfer.ts, and all three methods run theirformatargument through it.It belongs there rather than in
src/clipboard/paste.tsbecause the type is decided bysetData(), not by its callers, so fixing it at the method covers every caller at once.copySelection()and anyDataTransfera user builds and hands touserEvent.paste()reach the same methods, and a change confined topaste.tswould have left theurlshorthand and the case handling broken for all of them.paste.tsis untouched and now yieldstext/plainon its own, which is what the added end to end test asserts.One part of the spec algorithm is deliberately left out.
getData('url')is also supposed to parse atext/uri-listbody down to its first URL, which is a separate step from format normalization and is not included here.tests/utils/dataTransfer/DataTransfer.tsgains asetDataandgetDataround trip through both shorthands, the same round trip written in mixed case, an overwrite where an item declared under one spelling is replaced by writing the other, and aclearData()call that removes an item by its shorthand.tests/clipboard/paste.tsgains the reported scenario, asserting that a paste event built from a string exposestext/plainintypesand returns the string fromgetData('text/plain'). Every added test fails on main and passes with this change.The whole Jest suite passes at 514 tests in 54 files,
npm run validatereports no type errors, andeslintreports no new problems on the changed files.Checklist