Remove the dropzone global attribute. #2402

pwnall · 2017-02-25T03:29:02Z

dropzone failed to get traction among browser implementers. Having it in the specification is confusing to Web developers who may attempt to use it, only to discover that it is not supported.

Safari and Chrome implemented a prefixed version, webkitdropzone. The prefixed version is going away from Chrome [1]. Other browser vendors have no objections to the attribute getting removed from the spec [2].

[1] https://www.chromestatus.com/feature/5718005866561536
[2] #2331

This fixes #2331.

domenic · 2017-02-25T03:36:17Z

Thanks for doing this!

It looks like there are some build errors; would you mind fixing so we can review more easily?

pwnall · 2017-02-25T03:37:11Z

@domenic On it! I'm still trying to figure out what the errors mean, but I will definitely not give up until I get a green build.

pwnall · 2017-02-25T03:53:56Z

@domenic Can you please take another look?

I was looking at the line of the first error, until I realized that the 2nd line number is smaller than the first. 😅

domenic · 2017-02-25T04:12:30Z

Build is passing now, so I'll be able to compile locally and do a check :). It's the weekend at this point though so I'll take a look Monday!

pwnall · 2017-02-25T07:35:05Z

Thank you very much!

domenic

A few minor things to fix, but overall looking great!

domenic · 2017-02-27T19:58:22Z

source

+    whether or not the drop target is potentially willing to accept the drop.</li>
+
+    <li>The <code data-x="event-dnd-dragover">dragover</code> event handler must specify what
+    feedback will be shown to the user.</li>


Since this is a non-normative section, we should not be using "must" (or similar words). Not sure on the best rephrasing... maybe "can"?

I tried a draft that simply removed the "must". WDYT?

Looks good!

domenic · 2017-02-27T20:00:59Z

source

-  give (e.g. "<code data-x="">move</code>" to indicate that the data will be moved).</p>
+  <ol>
+
+    <li>The <code data-x="event-dnd-dragenter">dragenter</code> event handler must report


It might be helpful to add something like "by canceling the event"

Done. Thank you for the suggestion! Thinking back to the first time I coded up drag-and-drop, more clarity here is super-helpful.

domenic · 2017-02-27T20:01:55Z

source

+  }
+  function dragOverHandler(event) {
+    event.dataTransfer.dropEffect = 'move';
+  }
  function dropHandler(event) {


The example below this one also needs updating, to stay in sync.

I tried out the full example, and (assuming I got it right) the code fragment below doesn't need upgrading.

Well, the issue is that the code fragment below is supposed to be a modification of the above one (which you modified) to simply add dragend handling. So since you added dragenter/dragover handling to the first fragment, the second fragment should get updated to match.

GitHub seems to fail at expanding the diff (perhaps because source is a large file), so you'll have to use a local checkout to look at the lines I'm referencing here. Sorry about that!

IIUC, you're referring to lines 74331-74345. They're an update of lines 74244-74261, which I haven't changed. Can you please explain me what I got wrong? Also, thank you for bearing with me!

No problem!

The issue is that lines 74331-74345 are meant to be exactly the same example as lines 74289-74322, but with the addition of also adding a dragend handler. That is, consider the sentence between the two examples, annotated:

For our example here [i.e., the example on lines 74289-74322], that means updating the original markup [shown in those lines] to handle that [dragend] event, [as shown below by modifying the original markup but also adding some dragend-specific stuff].

This it's important that lines 74331-74345 match lines 74289-74322, except:

They should add dragend handling

They can use "... as before ..." to avoid repetition. (I might just do // dragEnterHandler, dragOverHandler, and dropHandler as before instead of the current function dragStartHandler(event) { // <em>...as before...</em> })

Clarified offline: I was confused. As @pwnall says, lines 74331-74345 are an update of lines 74244-74261, not an update of lines 74289-74322 like I thought.

pwnall · 2017-02-28T02:38:13Z

@domenic Thank you very much for your thoughtful feedback! PTAL?

pwnall · 2017-03-02T20:24:34Z

@domenic Thank you very much for helping me with my first contribution!

domenic · 2017-03-02T20:27:08Z

This LGTM! I forgot to ask though, we are trying to get web platform tests before merging any changes. I'm happy to help with that if you want, just let me know. Mostly it will be a matter of:

deleting html/editing/dnd/the-dropzone-attribute/
Removing any other tests that depend on dropzone; https://github.com/w3c/web-platform-tests/search?p=7&q=dropzone&utf8=%E2%9C%93 shows a few
Adding a line to https://github.com/w3c/web-platform-tests/blob/master/html/dom/elements/elements-in-the-dom/historical.html

pwnall · 2017-03-02T20:30:11Z

@domenic I started working on WPT.

I didn't remove everything with dropzone because I suspect that some other APIs (e.g. DataTransfer.dropEffect) might only be covered in tests that have dropzone. This was definitely the case in Blink. Should I ping this PR when I've removed everything?

domenic · 2017-03-02T20:32:52Z

Hmm, interesting. How long do you think that would take? I don't think we should hold up merging this much longer. So I think it would be fine to merge this with even just a small test addition to https://github.com/w3c/web-platform-tests/blob/master/html/dom/elements/elements-in-the-dom/historical.html and an open issue on the web-platform-tests repository tracking the removal of the rest.

zcorpan · 2017-03-03T09:53:48Z

dropzone has existed in the spec since 2010, so it seems reasonable to add it to the Obsolete section (with id="the-dropzone-attribute").

Otherwise LGTM.

webkitdropzone is a prefixed version of the dropzone HTML global attribute. dropzone failed to gain traction among browser vendors, and only Blink and WebKit implement prefixed versions. Therefore, dropzone is being removed from the HTML specification in whatwg/html#2402 The following LayoutTests, which depended on webkitdropzone, have been rewritten to use event listeners. These tests cover other drag-and-drop functionality that we still want covered. * fast/events/dropzone-001 -> fast/dnd/dropEffect-for-effectAllowed * fast/events/dropzone-002 -> fast/dnd/dropEffect-for-image * fast/events/dropzone-003 -> fast/dnd/dropEffect-for-link * fast/events/dropzone-004 -> fast/dnd/dropEffect-for-file The followng LayoutTests have been removed, because they do not cover additional functionality that we still need. * fast/events/dropzone-005 -> only covers dropzone-specific code * fast/dnd/file-drop-on-webkitdropzone-element.html -> redundant with fast/dnd/file-drag-drop-on-page.html * http/tests/dnd/file-drop-on-webkitdropzone-element.html -> redundant with fast/dnd/file-drag-drop-on-page.html BUG=688943 Review-Url: https://codereview.chromium.org/2720463002 Cr-Commit-Position: refs/heads/master@{#454488}

webkitdropzone is a prefixed version of the dropzone HTML global attribute. dropzone failed to gain traction among browser vendors, and only Blink and WebKit implement prefixed versions. Therefore, dropzone is being removed from the HTML specification in whatwg/html#2402 The following LayoutTests, which depended on webkitdropzone, have been rewritten to use event listeners. These tests cover other drag-and-drop functionality that we still want covered. * fast/events/dropzone-001 -> fast/dnd/dropEffect-for-effectAllowed * fast/events/dropzone-002 -> fast/dnd/dropEffect-for-image * fast/events/dropzone-003 -> fast/dnd/dropEffect-for-link * fast/events/dropzone-004 -> fast/dnd/dropEffect-for-file The followng LayoutTests have been removed, because they do not cover additional functionality that we still need. * fast/events/dropzone-005 -> only covers dropzone-specific code * fast/dnd/file-drop-on-webkitdropzone-element.html -> redundant with fast/dnd/file-drag-drop-on-page.html * http/tests/dnd/file-drop-on-webkitdropzone-element.html -> redundant with fast/dnd/file-drag-drop-on-page.html BUG=688943 Review-Url: https://codereview.chromium.org/2720463002 Cr-Commit-Position: refs/heads/master@{#454488} (cherry picked from commit 798a0d0) Review-Url: https://codereview.chromium.org/2729353002 . Cr-Commit-Position: refs/branch-heads/3029@{#9} Cr-Branched-From: 939b32e-refs/heads/master@{#454471}

dropzone failed to get traction among browser implementers. Having it in the specification is confusing to Web developers who may attempt to use it, only to discover that it is not supported. Safari and Chrome implemented a prefixed version, webkitdropzone. The prefixed version is going away from Chrome [1]. Other browser vendors have no objections to the attribute getting removed from the spec [2]. [1] https://www.chromestatus.com/feature/5718005866561536 [2] whatwg#2331

pwnall · 2017-03-06T19:38:42Z

@zcorpan I added dropzone to the Obsolete section. Can you please take a look and check that I used appropriate text?

dropzone is getting removed from the HTML standard in whatwg/html#2402

pwnall · 2017-03-06T19:59:52Z

@domenic Thank you very much for your pragmatic advice!

I submitted web-platform-tests/wpt#5052 for adding dropzone to the historical checks, and filed web-platform-tests/wpt#5053

Let's leave the massive "use CSS instead of all these presentational attributes" section at the bottom.

domenic · 2017-03-06T20:06:08Z

It all looks good to me! I pushed a small tweak to move it before all the "use CSS instead" items. I'll merge after the CI completes!

dropzone is getting removed from the HTML standard in whatwg/html#2402.

pwnall · 2017-03-06T21:04:13Z

Thank you very much for the feedback and for all the help & patience!

pwnall force-pushed the rm-dropzone branch from 0b4a708 to 98e05fe Compare February 25, 2017 03:51

domenic requested changes Feb 27, 2017

View reviewed changes

pwnall force-pushed the rm-dropzone branch from 98e05fe to 405fac3 Compare February 28, 2017 02:24

pwnall force-pushed the rm-dropzone branch from 405fac3 to 3f9c6d7 Compare March 2, 2017 20:19

domenic approved these changes Mar 2, 2017

View reviewed changes

domenic added the removal/deprecation Removing or deprecating a feature label Mar 2, 2017

domenic added the needs tests Moving the issue forward requires someone to write tests label Mar 2, 2017

pwnall force-pushed the rm-dropzone branch from 3f9c6d7 to 1d22192 Compare March 6, 2017 19:37

pwnall added a commit to pwnall/web-platform-tests that referenced this pull request Mar 6, 2017

Add dropzone to the list of obsoleted HTML elements.

3c1af3e

dropzone is getting removed from the HTML standard in whatwg/html#2402

This was referenced Mar 6, 2017

Add dropzone to the list of obsoleted HTML elements. web-platform-tests/wpt#5052

Merged

Remove dropzone tests web-platform-tests/wpt#5053

Open

Tweak position of the obsolete entry

ae55e12

Let's leave the massive "use CSS instead of all these presentational attributes" section at the bottom.

domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 6, 2017

Add dropzone to the list of obsoleted HTML elements

ac8cabb

dropzone is getting removed from the HTML standard in whatwg/html#2402.

domenic merged commit 3044a7e into whatwg:master Mar 6, 2017

stevefaulkner mentioned this pull request Mar 30, 2017

dropzone isn't supported w3c/html#64

Closed

Arttse mentioned this pull request Jan 16, 2018

Attribute dropzone was removed validator/validator#599

Closed

pwnall deleted the rm-dropzone branch May 2, 2018 02:43

whatwg deleted a comment from cklim483 Dec 21, 2022

whatwg deleted a comment from uzkjzjjz Apr 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the dropzone global attribute. #2402

Remove the dropzone global attribute. #2402

pwnall commented Feb 25, 2017

domenic commented Feb 25, 2017

pwnall commented Feb 25, 2017

pwnall commented Feb 25, 2017

domenic commented Feb 25, 2017

pwnall commented Feb 25, 2017

domenic left a comment

domenic Feb 27, 2017

pwnall Feb 28, 2017

domenic Feb 28, 2017

domenic Feb 27, 2017

pwnall Feb 28, 2017

domenic Feb 27, 2017

pwnall Feb 28, 2017

domenic Feb 28, 2017

pwnall Feb 28, 2017

domenic Mar 2, 2017

domenic Mar 2, 2017

pwnall commented Feb 28, 2017

pwnall commented Mar 2, 2017

domenic commented Mar 2, 2017 •

edited

Loading

pwnall commented Mar 2, 2017

domenic commented Mar 2, 2017

zcorpan commented Mar 3, 2017

pwnall commented Mar 6, 2017

pwnall commented Mar 6, 2017

domenic commented Mar 6, 2017

pwnall commented Mar 6, 2017

Remove the dropzone global attribute. #2402

Remove the dropzone global attribute. #2402

Conversation

pwnall commented Feb 25, 2017

domenic commented Feb 25, 2017

pwnall commented Feb 25, 2017

pwnall commented Feb 25, 2017

domenic commented Feb 25, 2017

pwnall commented Feb 25, 2017

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pwnall commented Feb 28, 2017

pwnall commented Mar 2, 2017

domenic commented Mar 2, 2017 • edited Loading

pwnall commented Mar 2, 2017

domenic commented Mar 2, 2017

zcorpan commented Mar 3, 2017

pwnall commented Mar 6, 2017

pwnall commented Mar 6, 2017

domenic commented Mar 6, 2017

pwnall commented Mar 6, 2017

domenic commented Mar 2, 2017 •

edited

Loading