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.

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

add read only mode of drag-data-store, fixes #95 #100

Merged
merged 3 commits into from Jul 13, 2019
Merged

Conversation

johanneswilm
Copy link
Contributor

@johanneswilm johanneswilm commented Jul 1, 2019

@johanneswilm
Copy link
Contributor Author

Hey @marcoscaceres ,
We have kept the V1 and V2 in sync as most fixes that are requested need to be applied to both. However now I noticed you have changed the formatting of V2 but not V1, so that we can no longer easily cherry-pick commits from the other version.That's a bit unfortunate, but hopefully V1 stops changing in the very near future.

@marcoscaceres
Copy link
Member

Oops, sorry about that. Let’s see if we can get them back in sync. It might be better if we look at getting rid of the levels. Our charter changed so we no longer allow specs with single implementation.

@johanneswilm
Copy link
Contributor Author

Our charter changed so we no longer allow specs with single implementation.

Does that mean that the implementations have to come first before one can write a spec about it? Level 1 is currently implemented in Chrome, level 2 in Safari. Level 2 was there first (it did not have any level name back then) and had already been implemented by Safari. Level 1 was created subsequently so that Chrome could implement just a part of the spec as they ran into problems with text input on Android. It was either create a partial spec (level 1) or not implement anything at all.

@frivoal
Copy link

frivoal commented Jul 2, 2019

Our charter changed so we no longer allow specs with single implementation.

Has it? As far as I can see, the charter does say:

In order to advance to Proposed Recommendation, each specification must have at least two independent implementations in wide use.

But that's 2 implementations to advance to PR, not two implementations to take up work at all.

It also says:

Additionally, support from two or more web platform implementers is required before a substantive change can be made to a specification.

However, this is about support from 2 implementers, which isn't the same as having 2 implementations already, and no part of the charter calls for removing from specs things that got WG support in the past (which includes implementers, so if we were in consensus, we arguably have support from implementers) but currently lack 2 implementations.

@marcoscaceres
Copy link
Member

We can nit pick the semantics of the charter, but the intention is pretty clear: we don’t want speculative stuff in specs, and we are trying to course correct from past mistakes. Ultimately, we want specs that reflect reality and get implemented (and implementation commitments). That’s what we are trying to do here.

If only one browser is committing to do implement something, that’s not a standard: don’t matter if it’s Safari, Chrome, or Firefox.

@johanneswilm
Copy link
Contributor Author

Discussion about this specification and the different levels as well as implementation work has been ongoing for several years and has been the main work of the editing taskforce. What is now known as level 2 was to a large extend been co-developed by Chrome devs, until they suddenly found out about an android problem. It is still unclear whether they are able to fix the android problem and eventually implement the current level 2, or whether we need to change it some more so that everyone can implement it. This remains to be seen and depends on factors outside of what we can control.

I don't think getting rid of a level is a helpful contribution to this process. Level 1 is what is ready to move to REC once all the tests are in place. Level 2 is the current state of the discussion and it even has an implementation already. Obviously, at the beginning of this process and for a long time, none of them had any implementations, so I don't understand how this can suddenly be a requirement for working on the specs.

@johanneswilm
Copy link
Contributor Author

It should also be noted that level 1 is borderline useless. It's like teaching kids the first half of the alphabet because there is a problem with the letter "M". So having level 1 implemented is a good step toward getting something like current level 2 or some other alternative to it, but in itself it likely will not get a lot of usage. There are a few cases where it will help a little bit when using it in conjunction with existing methods, just like there are a few words and sentences you an write by only using the letters A-L. But if it were just that, it probably wouldn't have been worth the effort.

@marcoscaceres
Copy link
Member

Level 1 is what is ready to move to REC once all the tests are in place.

That’s awesome to hear.

Level 2 is the current state of the discussion and it even has an implementation already.

Ok, the README needs an update then. It reads like, “only Safari implements this.”

It should also be noted that level 1 is borderline useless.

Don’t think that way. Getting any interop is a great achievement and should not be under appreciated.

Baby steps will get us L2 - but we gotta get interop where ever we can. Every little bit of interop is a win for the Web, even if we don’t get all the way to Z.

@marcoscaceres
Copy link
Member

About:

I don't think getting rid of a level is a helpful contribution to this process.

If what you say holds with regards to L1 (and we can get that to Rec), then I concur. The README lead me to believe that L2 was just documenting Safari behavior. Now, if stuff in L2 does have consensus, and is implement, we could even consider bringing that stuff to L1, no? We don’t need to, but gives us some potential standardization wins.

@johanneswilm
Copy link
Contributor Author

Level 2 is the current state of the discussion and it even has an implementation already.

Ok, the README needs an update then. It reads like, “only Safari implements this.”

Oh! Yeah, if it can be misinterpreted that way, then yes, it should be reworded.

It should also be noted that level 1 is borderline useless.

Don’t think that way. Getting any interop is a great achievement and should not be under appreciated.

Baby steps will get us L2 - but we gotta get interop where ever we can. Every little bit of interop is a win for the Web, even if we don’t get all the way to Z.

I agree that it's good that we get L 1 to start with. What I am referring to is that this spec was supposed to make life easier for JS text editor developers. Text editors already exist on the web and they use a variety of other methods through which they interact with the browser. L 1 of Input Events doesn't really give them much more than what they have already and for some things even less, so they are likely to stick to the existing methods for most of their work (with the exception of a few borderline cases). So just removing level 2 and with it all the enhancements compared to level 1 that we have in there doesn't sound like a good idea to me.

@johanneswilm
Copy link
Contributor Author

If what you say holds with regards to L1 (and we can get that to Rec), then I concur. The README lead me to believe that L2 was just documenting Safari behavior. Now, if stuff in L2 does have consensus, and is implement, we could even consider bringing that stuff to L1, no? We don’t need to, but gives us some potential standardization wins.

The situation was this: The current Level 2 had consensus, although we had no level differentiation back then. Chrome and Safari started implementing it. Safari finished and shipped it, while Chrome got stuck on an Android issue and eventually we were presented with an ultimatum by the Chromium/Android team: Either we create a version of the spec that only covers half of level 2, or Chrome is not able to implement any of it for the time being. So we agreed to make this half-spec and called it "level 1" (and renamed the spec we had been working on "level 2"). I believe the reason they needed that half-spec was that the Chrome-internal policy at the time was they could only implement things that were written in a draft spec, and they could only implement the entire spec or none of it. At least that was my understanding of their argument why we needed to create level 1. Unless some other browser has the same Android problems, there is really no good reason to implement level 1, except Chrome's current market share and wanting to be entirely compatible with Chrome.

So this is the reason why we cannot bring anything to level 1 that Chrome didn't implement - or maybe having their own spec doesn't matter any more given that they already shipped it. I don't really know, but given that we are so close with level 1 and have gone through all the mandatory reviews I'd prefer to just ship that first.

There has been outspoken support for level 2 also from Edge, although I guess that doesn't count any more. Firefox has not yet implemented either, but will likely start with level 1.

Since then there have been a number of minor issues and things to do with level 1, but now that it's only the tests, my plan is to have that go to REC. And then we go back and look at level 2 and see if either Android has been fixed or whether we can continue with that in some other direction that works for everyone.

@marcoscaceres
Copy link
Member

So just removing level 2 and with it all the enhancements compared to level 1 that we have in there doesn't sound like a good idea to me.

Ah, I think I get where we are getting our wires crossed: the fact that there are words in a document doesn’t suddenly make those words a Standard. It doesn’t matter how great those ideas are or if they solve real problems: Without multiple implementations, or serious commitment from multiple implementers, and tests, the words are just conjecture. Standards should be documenting real, implemented, fully tested things. Otherwise, we end up in this situation: frustration because good ideas in what seems like a standard don’t get implemented or the attention they deserve.

@marcoscaceres
Copy link
Member

So this is the reason why we cannot bring anything to level 1 that Chrome didn't implement - or maybe having their own spec doesn't matter any more given that they already shipped it. I don't really know, but given that we are so close with level 1 and have gone through all the mandatory reviews I'd prefer to just ship that first.

Sounds good to me.

There has been outspoken support for level 2 also from Edge, although I guess that doesn't count any more.

It definitely counts, specially if the add stuff the Blink that matches WebKit. Remember, it’s “engines”, not browsers that count.

Firefox has not yet implemented either, but will likely start with level 1.

😢

Since then there have been a number of minor issues and things to do with level 1, but now that it's only the tests, my plan is to have that go to REC. And then we go back and look at level 2 and see if either Android has been fixed or whether we can continue with that in some other direction that works for everyone.

This sounds like a great plan. Let’s get this done ✅

@johanneswilm
Copy link
Contributor Author

Ah, I think I get where we are getting our wires crossed: the fact that there are words in a document doesn’t suddenly make those words a Standard. It doesn’t matter how great those ideas are or if they solve real problems: Without multiple implementations, or serious commitment from multiple implementers, and tests, the words are just conjecture. Standards should be documenting real, implemented, fully tested things. Otherwise, we end up in this situation: frustration because good ideas in what seems like a standard don’t get implemented or the attention they deserve.

I think you are talking about documenting things that have already been implemented and sort of been developed over time without much of a plan. That probably works for a lot of things, but in the area of editing we found that this didn't work well in the case of execCommand which ended up not being used a lot and implementations differing in many different ways to a degree where it now seems impossible to get them to behave the same now. So we tried a different approach of sitting together and working out the spec and then simultaneously implementing it in a way that actually makes it possible to build applications on it across browsers. And then suddenly there was Android.

But anyway, it sounds like we agree on the immediate steps going forward.

index.html Outdated
@@ -1288,6 +1284,10 @@ <h5>Attributes</h5>
<td>
A prepopulated <a>DataTransfer</a> object so that:
<ol>
<li>The <a>DataTransfer</a> object's <a data-cite="html/dnd.html#drag-data-store">drag data store</a> is in
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<li>The <a>DataTransfer</a> object's <a data-cite="html/dnd.html#drag-data-store">drag data store</a> is in
<li>The {{DataTransfer}} object's <a data-cite="html/dnd.html#drag-data-store">drag data store</a> is in

Probably want to update globally... you can use {{ WhatEver }} when talking about WebIDL things.

index.html Outdated
@@ -1288,6 +1284,10 @@ <h5>Attributes</h5>
<td>
A prepopulated <a>DataTransfer</a> object so that:
<ol>
<li>The <a>DataTransfer</a> object's <a data-cite="html/dnd.html#drag-data-store">drag data store</a> is in
<a data-cite="html/dnd.html#drag-data-store">read-only</a>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<a data-cite="html/dnd.html#drag-data-store">read-only</a>
<a data-cite="html/dnd.html#concept-dnd-ro">read-only</a>

@marcoscaceres
Copy link
Member

Left a couple of suggestions, but overall looks fine.

@marcoscaceres
Copy link
Member

We have kept the V1 and V2 in sync as most fixes that are requested need to be applied to both. However now I noticed you have changed the formatting of V2 but not V1, so that we can no longer easily cherry-pick commits from the other version.That's a bit unfortunate, but hopefully V1 stops changing in the very near future.

I've fixed up the v1 branch, and tidy'ed it up. Only contains the differences, so hopefully cherry-picks should apply.

@marcoscaceres
Copy link
Member

As I fixed up v1, you might need to redo this PR (thankfully it was quite small). After that, it should be good.

@johanneswilm johanneswilm merged commit f651119 into v1 Jul 13, 2019
@johanneswilm johanneswilm deleted the drag-data-store-v1 branch July 13, 2019 20:48
janiceshiu pushed a commit to janiceshiu/input-events that referenced this pull request Feb 24, 2020
GlobalEventHandlers is a mixin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants