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
The ClipboardEvent constructor is strange #10
Comments
CC @Tanayc |
Ping @hallvors, any idea what we should do with this issue? |
Hi @foolip, it's correct that Gecko hasn't implemented the spec as it is now. Thanks for the implementor feedback - this part of the spec used to have createEvent() .. e.initEvent( ... ) stuff and was changed on Anne's request, so you are most likely the first implementor to read it really closely since it was rewritten. Sorry I missed it initially - GitHub sends way too much E-mail.. ;) BTW it sounds like you're writing some tests while implementing - would be great if you could help reviewing these web-platform-tests/wpt#1242 and give feedback - for example report bugs against the spec on stuff that's not covered by those tests.
Not quite sure what "this" refers to, but maybe it doesn't matter..
Good point - nothing in the spec says explicitly that when you do
That algorithm should (AFAIK) handle a scripted dispatch too though, as it makes an effort to get things like isTrusted right. What you're missing is some line somewhere saying that if dispatchEvent() is called with an event object whose constructor is ClipboardEvent, the implementation should run the "fire a clipboard event" steps? So - if we add some text describing the construction of a clipboard event object with a DataTransfer property, and a line about dispatchEvent you'll be happy? :) |
Sorry, lost some context, that was when commenting on the constructor, and the "this" that's different is how the spec just references the arguments of the constructor at a later time, while an implementation would do it in the constructor, and actually firing the event wouldn't modify any object anywhere. |
I filed an HTML bug on a similar issue yesterday: We should have the same pattern in both cases. |
That would amount to monkey patching of DOM's dispatch, and I don't think any other spec adds special steps to run as part of that algorithm, at least in Blink's implementation I see nothing like it. CC @annevk I can see two solutions:
|
If we can avoid changing the dispatch algorithm that would be good. |
Is possible to actually do anything useful with the |
Not sure bout DragEvent, but I think a complex app with some internal copy/cut/paste stuff, or a library that aims to work in a generic way could hook into other parts of the app's copy/paste logic with synthetic events. To fix this issue I'll drop the second argument to the constructor and add an example using items.add(). But I should also look at the "fire an event" stuff and perhaps move some of the early steps to an "initialize a clipboard event" section. |
@foolip would be great if you could check those changes, and the corresponding testsuite fixes. Happy? :) |
You still need to take a second argument, probably just |
IDL is updated to have a second optional EventInit argument. |
Thanks! |
Before we go ahead and implement that in Blink, is there someone working on the implementation in Gecko that can confirm that this is OK? |
You want @smaug---- and @ehsan to r+ this from our side. |
Thanks, I patiently await their judgment :) |
Not sure what I should comment about here. (off topict, it is partially because of consistency in Event interfaces which lets Gecko to autogenerate C++ implementations from most of the *Event interfaces) |
If so, what should be its members? It's not possible to create a |
in JS, sure. It is not possible to create DataTransfer atm (maybe we should add a ctor for DataTransfer but that is a different bug). other option is to use 'required' in the dictionary. |
Is this on principle, that because If the only way to get a In any case, |
Ping @smaug----. There's an Intent to Implement and Ship DragEvent on blink-dev which will be blocked on bug 28984 which will in turn be blocked on this issue. |
er, I meant clipboardData in my comment, not dataTransfer. And yes, it is on principle that since (I do assume we'll want to make DataTransfer constructible at some point.) |
The |
Why? It seems fine that you cannot really construct an instance in most cases. There's a few other such scenarios floating around. Maybe down the line it'll be easier. |
Nullable stuff seems generally bad unless there's a good case for them. |
@annevk, @smaug----, do you care which of the options in #10 (comment) prevails, or is anything fine at this point? |
I don't like 2. It seems @smaug---- has a slight preference for 3, which I don't particularly mind. |
Going with option 3 would mean closing https://www.w3.org/Bugs/Public/show_bug.cgi?id=28984 as won't fix, which means less work. @smaug----, are you OK with this? |
|
Do you really mean 1 as in no |
Er, I mean 3. |
Great, so I think the only remaining change is to make |
Hm.. If it's nullable I need some extra text saying when it's null or not? Or will going with option 3 mean (per Philip's comment in https://critic.hoppipolla.co.uk/showcomment?chain=12258 ) that
won't work because evt.clipboardData is null? That will make synthetic paste events pretty useless.. |
That would be option 2 from #10 (comment), which @annevk doesn't like. |
Synthetic events are generally useless. They're a debugging aid. |
Even if it's only used for writing tests I'd consider that a somewhat important use case ;) I also wanted to make synthetic paste events with actual data payloads functional so that scripts that want to preserve modularity could use synthetic paste events for data exchange. For example, say I write a user JS / Greasemonkey script that keeps a library of "clip art" and can insert images into any editor component capable of handling a paste, or a "snippets" type library that can run on any page, send XHR to the user's private service to load their stored snippets and insert them on demand in any context that has scripts listening to paste events.. I think we make some neat things possible or simpler here. |
(But I admit that I don't really understand why there needs to be such a strong connection between the event init dict and the properties on the event object - why there needs to be a non-nullable clipboardData property on ClipboardEventInit in order to have an always present clipboardData property on the event object. Event object properties are just copied automatically from EventInit? We can't just say ClipboardEvent IDL says that there is a DataTransfer clipboardEvent property, so if you create a ClipboardEvent you must add this property ..??) |
@hallvors a synthetic event cannot cause actions... How often do we need to have that conversation? I tried to clarify that here: https://dom.spec.whatwg.org/#action-versus-occurance |
@annevk it's not supposed to cause actions, it is supposed to trigger listeners that carry out actions. Sorry if I'm being unclear. By "editor capable of handling a paste" and "context that has scripts listening to paste events" I mean DOM elements (whether contentEditable=true/designMode=on or plain TEXTAREA/INPUT) that have a 'paste' event listener that will respond to paste events and do something with the event's data payload. Hence, if a synthetic event can have a payload it can be used both for testing and for data exchange between components that are designed to avoid tighter coupling. If a synthetic paste event can not have a payload it's pretty much useless and we might as well drop the constructor and avoid the issue. Clear enough? (BTW I think I remember making a minor edit right after submitting the comment, perhaps Github E-mailed you the first version where the last sentence was less clear. Apologies for any confusion.) |
Yes, per a macro.
There's a couple things wrong with this question, but the main one is that you cannot really say it creates a |
Ping? @Tanayc is kind of blocked on this issue, what's needed to reach a conclusion here? |
@foolip and @Tanayc - sorry about the lack of follow-up, I missed Phillip's ping.. So we're not done with this discussion yet, right? How is this done for DragEventInit.dataTransfer ? ClipboardEventInit.clipboardData should probably do exactly the same thing as it's very similar. I guess I have to give up the ambition of having synthetic events that actually behave a bit like real events and can be of some use.. :-/ |
I made a minor change to align |
@foolip that's all, I think.. I wonder why we don't have a DataTransfer constructor. It might be a good way to improve the clipboard API overall - it's a topic we're starting to discuss now.. Perhaps we'll decide to add a constructor and we could revisit this and make sure we can pass in an actual DataTransfer instance to the ClipboardEvent constructor some day :) |
I agree, with a I commented about a missing |
I just need to demonstrate my lacking understanding of WebIDL :/ |
Wait, why did you drop |
Because the output (as in the spec text you read) when I add the
And I don't know why respec doesn't like =null while you do because I'm trying to make something else work and wanted to close this in no time :-p.. |
Sorry, I didn't look closely and thought I was looking at a |
FYI: I'm in the process of adding a constructor for DataTransfer because it is needed for the Async Clipboard API. I mention this just in case anyone wants to revisit the nullable clipboardData issue. Tracking issue: whatwg/html#2190 |
I'd say it's probably best to leave them nullable, because we'd also have to make those dictionary members required in the constructor, causing any existing code like |
https://w3c.github.io/clipboard-apis/#clipboard-event-interfaces
There's an effort to implement the
ClipboardEvent
constructor in Blink:https://code.google.com/p/chromium/issues/detail?id=496394
https://codereview.chromium.org/1178683007/
In the review I noticed some weird things, quoted here:
How should we reach interoperability here? To me the
data
anddataType
members don't seem very useful, and it would be more in line with other event constructors to simply take aclipboardData
attribute. IfDataTransfer
doesn't have a constructor, maybe it should? If it shouldn't, then perhaps aClipboardEvent
constructor doesn't make sense either.The text was updated successfully, but these errors were encountered: