Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2572023002:
Remove special DataTransfer permissions for file: origins. (Closed)
DescriptionRemove special DataTransfer permissions for file: origins.
Blink currently uses an overly permissive mode for DataTransfer
instances given to the dragenter/dragover/dragleave event handlers of a
page served from a file: origin. According to the specification, the
DataTransfer instance should only allow limited metadata to be read.
Blink allows the page to read all the drag information.
This change aligns the DataTransfer mode used in these event handlers to
the current drag and drop specification.
BUG=104681
Committed: https://crrev.com/2cb1858d033ef1154182ca529e16703693ae9cf6
Cr-Commit-Position: refs/heads/master@{#439627}
Patch Set 1 : Pulled webkitdropzone changes to separate CL. #
Total comments: 2
Patch Set 2 : Feedback from dcheng. #
Total comments: 29
Patch Set 3 : jsbell feedback. #Patch Set 4 : jsbell feedback, round 2. #
Depends on Patchset: Messages
Total messages: 60 (47 generated)
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #1 (id:40001) has been deleted
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:60001) has been deleted
Description was changed from ========== Remove special-case DataTransfer mode for file: origins in dragenter/dragover. This change aligns the DataTransfer mode to the current drag and drop specification. Blink previously gave extra information to pages served from a file: origin. BUG=104681 ========== to ========== Remove special DataTransfer permissions for file: origins. Blink currently uses an overly permissive mode for DataTransfer instances given to the dragenter/dragover/dragleave event handlers of a page served from a file: origin. According to the specification, the DataTransfer instance should only allow limited metadata to be read. Blink allows the page to read all the drag information. This change aligns the DataTransfer mode used in these event handlers to the current drag and drop specification. BUG=104681 ==========
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:80001) has been deleted
pwnall@chromium.org changed reviewers: + dcheng@chromium.org
PTAL?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The code change LG, but I'm not familiar enough with all the new promise stuff in testharness. Is there documentation I can read about this? (In the meanwhile, I'm happy to LG the code change if someone else who's more familiar with testharness.js is willing to review the layout test changes) https://codereview.chromium.org/2572023002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/100001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:62: dropWrapper.ondrop = (event) => event.preventDefault(); Why do we need to install listeners on the wrapper and the dropzone? It's true that it won't be considered a drop over the wrapper, but I think we only care about the dropzone.
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/16 18:41:57, dcheng wrote: > The code change LG, but I'm not familiar enough with all the new promise stuff > in testharness. Is there documentation I can read about this? http://testthewebforward.org/docs/testharness-library.html#promise-tests This is the first document that comes to mind, as it covers the basic API. Is this what you were asking about? > (In the meanwhile, I'm happy to LG the code change if someone else who's more > familiar with testharness.js is willing to review the layout test changes) I'd be happy to get another reviewer for the tests if you don't have time to go through the docs. Thank you for the quick turnaround!
https://codereview.chromium.org/2572023002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/100001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:62: dropWrapper.ondrop = (event) => event.preventDefault(); On 2016/12/16 18:41:57, dcheng wrote: > Why do we need to install listeners on the wrapper and the dropzone? It's true > that it won't be considered a drop over the wrapper, but I think we only care > about the dropzone. Ahh, good point. Thank you very much for catching this! I was mostly in auto-pilot mode after having to set up duplicate listeners for all the other events (which are needed). I was thinking of event propagation and navigation drops, but calling preventDefault() once is sufficient.
pwnall@chromium.org changed reviewers: + jsbell@chromium.org
jsbell: Can you please review the layout tests?
code changes lgtm
Comments in the first test file apply to the others as well. lgtm with various nits and suggestions https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:40: dropWrapper.ondragenter = (event) => event.preventDefault(); nit: no () needed around single argument (here and below) https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:40: dropWrapper.ondragenter = (event) => event.preventDefault(); Something we should decide on in the style guide: for arrow funcs where the intent is a side effect and the return value is ignored, would we prefer: e => e.preventDefault() or: e => { e.preventDefault(); } I personally prefer the latter but (as noted) it should be a style guide thing. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:68: const wrapperRect = dropWrapper.getBoundingClientRect(); These can move into the if block. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:85: }, 16); Why 16? https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:90: assert_equals(JSON.stringify(dataTransfer.types), '["Files"]'); Will this work? assert_array_equals(dataTransfer.types, ['Files']); If not, will this? assert_array_equals(Array.from(dataTransfer.types), ['Files']); (I'm not sure how nitpicky assert_array_equals is) https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:92: }, 'dragenter types'); Can you clarify - in either the test names or assertion messages - why the expected results are expected? e.g. assert_array_equals(dataTransfer.types, ['Files'], 'dragenter's data should contain types'); ... and then below, that data['Files'] should always be empty and files should be empty until the drop itself. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:138: assert_equals(JSON.stringify(dataTransfer.files), '[]'); Can you assert that length is 0 rather than using JSON.stringify ? https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:6: // getData function. ... and the contents of files via FileReaders. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:20: // DataTransfer.files returns a FileList. According to the spec, FileList only Consider using Array.from(dataTransfer.files || []) if you want something you can iterate/map over more easily. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:33: fileData.data = event.target.error; assigning to .data rather than .error here - intentional? https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:42: // DataTransfer.items returns an DataTransferItemList, which supports .length Same as above, re: Array.from() https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:73: return { data: data, files: files, items: items, types: types }; Style guide issue: can we start using simplified initializer notation? return { data, files, items, types }; This is in ES2015, so just as old as arrow functions. Google's ES6 guide says yes: https://google.github.io/styleguide/jsguide.html#features-objects-shorthand-p... but Chromium may vary.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
jsbell: PTAL? I think I've addressed much of your feedback, and I asked a couple of questions. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:40: dropWrapper.ondragenter = (event) => event.preventDefault(); On 2016/12/16 21:16:15, jsbell wrote: > nit: no () needed around single argument (here and below) Done. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:40: dropWrapper.ondragenter = (event) => event.preventDefault(); On 2016/12/16 21:16:15, jsbell wrote: > nit: no () needed around single argument (here and below) Done. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:68: const wrapperRect = dropWrapper.getBoundingClientRect(); On 2016/12/16 21:16:15, jsbell wrote: > These can move into the if block. I have them outside so I can see typos in the console when I run the tests manually. I generally try to have as little code as possible in "if (window.eventSender)", because it's much harder to debug. Would you still like me to move the code in the if block, or would it be OK to leave it where it is? https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:85: }, 16); On 2016/12/16 21:16:15, jsbell wrote: > Why 16? I wanted to have the move go in a different frame (at 60fps), but that doesn't really matter for us. I switched the timeout to 0, which is sufficient to have dragleave occur in a different task. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:90: assert_equals(JSON.stringify(dataTransfer.types), '["Files"]'); On 2016/12/16 21:16:15, jsbell wrote: > Will this work? > > assert_array_equals(dataTransfer.types, ['Files']); > > If not, will this? > > assert_array_equals(Array.from(dataTransfer.types), ['Files']); > > (I'm not sure how nitpicky assert_array_equals is) Done. FWIW, I had assert_array_equals at first, but the error message is much less informative. If the arrays have different lengths, it'll just say that, without showing the array contents. I found the JSON form to be more helpful for understanding how Firefox differs from us. Then again, I suppose this is a testharness.js issue. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:92: }, 'dragenter types'); On 2016/12/16 21:16:15, jsbell wrote: > Can you clarify - in either the test names or assertion messages - why the > expected results are expected? e.g. > > assert_array_equals(dataTransfer.types, ['Files'], > 'dragenter's data should contain types'); > > ... and then below, that data['Files'] should always be empty and files should > be empty until the drop itself. Done. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:138: assert_equals(JSON.stringify(dataTransfer.files), '[]'); On 2016/12/16 21:16:15, jsbell wrote: > Can you assert that length is 0 rather than using JSON.stringify ? Done. I guess I don't have the same issue here as before -- knowing what's in the array if it's not empty isn't very useful for debugging. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:6: // getData function. On 2016/12/16 21:16:16, jsbell wrote: > ... and the contents of files via FileReaders. Done. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:20: // DataTransfer.files returns a FileList. According to the spec, FileList only On 2016/12/16 21:16:16, jsbell wrote: > Consider using Array.from(dataTransfer.files || []) if you want something you > can iterate/map over more easily. Would this work cross-browser? Array.from() wants an array-like object (.length and []) or an iterable. The drag-and-drop spec states that DataTransfer.files returns a FileList. This is spec'd in the File API TR as an object with .length and .item(index). Notably, the [] operator is missing. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:33: fileData.data = event.target.error; On 2016/12/16 21:16:16, jsbell wrote: > assigning to .data rather than .error here - intentional? Done. Thanks for catching this! It helped while developing, but I think .error makes more sense given the current asserts. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:42: // DataTransfer.items returns an DataTransferItemList, which supports .length On 2016/12/16 21:16:16, jsbell wrote: > Same as above, re: Array.from() Done. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:73: return { data: data, files: files, items: items, types: types }; On 2016/12/16 21:16:16, jsbell wrote: > Style guide issue: can we start using simplified initializer notation? > > return { data, files, items, types }; > > This is in ES2015, so just as old as arrow functions. Google's ES6 guide says > yes: > https://google.github.io/styleguide/jsguide.html#features-objects-shorthand-p... > but Chromium may vary. Done. AFAIK , Chromium's style guide still uses the old ES5 Google style guide as a baseline, because of build pipeline issues (e.g., Closure). My layout tests writeup uses the new ES6 style guide as a baseline, because layout tests don't use any build pipeline, so the issues aren't relevant. So, this should work. Thanks for pointing out this ES6 feature! I'm still wrapping my head around it.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm You could optionally switch to Array.from() per below, but not a big deal. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:68: const wrapperRect = dropWrapper.getBoundingClientRect(); On 2016/12/17 00:44:48, pwnall wrote: > On 2016/12/16 21:16:15, jsbell wrote: > > These can move into the if block. > > I have them outside so I can see typos in the console when I run the tests > manually. I generally try to have as little code as possible in "if > (window.eventSender)", because it's much harder to debug. > > Would you still like me to move the code in the if block, or would it be OK to > leave it where it is? OK to leave as is. https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:90: assert_equals(JSON.stringify(dataTransfer.types), '["Files"]'); On 2016/12/17 00:44:48, pwnall wrote: > Then again, I suppose this is a testharness.js issue. Agreed. I think the tests should focus on expressing intent and the harness should make the output usable. File a testharness bug? https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:20: // DataTransfer.files returns a FileList. According to the spec, FileList only On 2016/12/17 00:44:48, pwnall wrote: > On 2016/12/16 21:16:16, jsbell wrote: > > Consider using Array.from(dataTransfer.files || []) if you want something you > > can iterate/map over more easily. > > Would this work cross-browser? > > Array.from() wants an array-like object (.length and []) or an iterable. The > drag-and-drop spec states that DataTransfer.files returns a FileList. This is > spec'd in the File API TR as an object with .length and .item(index). Notably, > the [] operator is missing. The spec has: getter File? item(unsigned long index); Which per WebIDL (https://heycam.github.io/webidl/#idl-special-operations) is the same as having both: getter File? (unsigned long index); File? item(unsigned long index); Where 'getter' defines the indexed property lookup behavior ('[] operator'). So this should be fine cross-browser.
The CQ bit was checked by pwnall@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jsbell: Thank you very much for helping me make the tests cleaner! https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/file-drag-drop-on-page.html:90: assert_equals(JSON.stringify(dataTransfer.types), '["Files"]'); On 2016/12/19 17:25:17, jsbell wrote: > On 2016/12/17 00:44:48, pwnall wrote: > > Then again, I suppose this is a testharness.js issue. > > Agreed. I think the tests should focus on expressing intent and the harness > should make the output usable. File a testharness bug? > Done. I filed https://github.com/w3c/testharness.js/issues/227 https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js (right): https://codereview.chromium.org/2572023002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/dnd/resources/copy-data-transfer.js:20: // DataTransfer.files returns a FileList. According to the spec, FileList only On 2016/12/19 17:25:17, jsbell wrote: > On 2016/12/17 00:44:48, pwnall wrote: > > On 2016/12/16 21:16:16, jsbell wrote: > > > Consider using Array.from(dataTransfer.files || []) if you want something > you > > > can iterate/map over more easily. > > > > Would this work cross-browser? > > > > Array.from() wants an array-like object (.length and []) or an iterable. The > > drag-and-drop spec states that DataTransfer.files returns a FileList. This is > > spec'd in the File API TR as an object with .length and .item(index). Notably, > > the [] operator is missing. > > The spec has: > > getter File? item(unsigned long index); > > Which per WebIDL (https://heycam.github.io/webidl/#idl-special-operations) is > the same as having both: > > getter File? (unsigned long index); > File? item(unsigned long index); > > Where 'getter' defines the indexed property lookup behavior ('[] operator'). > > So this should be fine cross-browser. Done. Thank you for explaining! I'll schedule a WebIDL spec reading in the nearby future.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by pwnall@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org, jsbell@chromium.org Link to the patchset: https://codereview.chromium.org/2572023002/#ps160001 (title: "jsbell feedback, round 2.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1482192101979590,
"parent_rev": "25f7128b3419b9d24b26e0d8169fa30036e6e6a9", "commit_rev":
"8e4801382e4983935a0fb112449d61aadc134b73"}
Message was sent while issue was closed.
Description was changed from ========== Remove special DataTransfer permissions for file: origins. Blink currently uses an overly permissive mode for DataTransfer instances given to the dragenter/dragover/dragleave event handlers of a page served from a file: origin. According to the specification, the DataTransfer instance should only allow limited metadata to be read. Blink allows the page to read all the drag information. This change aligns the DataTransfer mode used in these event handlers to the current drag and drop specification. BUG=104681 ========== to ========== Remove special DataTransfer permissions for file: origins. Blink currently uses an overly permissive mode for DataTransfer instances given to the dragenter/dragover/dragleave event handlers of a page served from a file: origin. According to the specification, the DataTransfer instance should only allow limited metadata to be read. Blink allows the page to read all the drag information. This change aligns the DataTransfer mode used in these event handlers to the current drag and drop specification. BUG=104681 Review-Url: https://codereview.chromium.org/2572023002 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Remove special DataTransfer permissions for file: origins. Blink currently uses an overly permissive mode for DataTransfer instances given to the dragenter/dragover/dragleave event handlers of a page served from a file: origin. According to the specification, the DataTransfer instance should only allow limited metadata to be read. Blink allows the page to read all the drag information. This change aligns the DataTransfer mode used in these event handlers to the current drag and drop specification. BUG=104681 Review-Url: https://codereview.chromium.org/2572023002 ========== to ========== Remove special DataTransfer permissions for file: origins. Blink currently uses an overly permissive mode for DataTransfer instances given to the dragenter/dragover/dragleave event handlers of a page served from a file: origin. According to the specification, the DataTransfer instance should only allow limited metadata to be read. Blink allows the page to read all the drag information. This change aligns the DataTransfer mode used in these event handlers to the current drag and drop specification. BUG=104681 Committed: https://crrev.com/2cb1858d033ef1154182ca529e16703693ae9cf6 Cr-Commit-Position: refs/heads/master@{#439627} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/2cb1858d033ef1154182ca529e16703693ae9cf6 Cr-Commit-Position: refs/heads/master@{#439627} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
