Git Product home page Git Product logo

Comments (65)

foolip avatar foolip commented on July 20, 2024

CC @Tanayc

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Ping @hallvors, any idea what we should do with this issue?

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

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.

the spec expresses this differently

Not quite sure what "this" refers to, but maybe it doesn't matter..

As part of "fire a clipboard event" it checks if the event is trusted and
uses the data and dataType arguments from the constructor without
actually storing somewhere in the interim.

Good point - nothing in the spec says explicitly that when you do new ClipboardEvent('paste', {dataType:'text/plain', data:'foo'}) you'll get an event object with a clipboardData property whose items list contains a single entry.

Also strange is that there's no way to actually run the "fire a clipboard event"
algorithm from scripts, all of the entry points look like they're for actual copy/paste/cut actions.

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? :)

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Not quite sure what "this" refers to, but maybe it doesn't matter.

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.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

I filed an HTML bug on a similar issue yesterday:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=28984

We should have the same pattern in both cases.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

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?

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:

  1. Add a DataTransfer constructor and make ClipboardEventInit like DragEventInit.
  2. Let ClipboardEventInit and DragEventInit have no dictionary members at all, and make the constructors create events with a DataTransfer instance with nothing in it. It looks like DataTransferItemList.add() can be used to populate it.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

If we can avoid changing the dispatch algorithm that would be good.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Is possible to actually do anything useful with the ClipboardEvent or DragEvent constructors? If it isn't could we just not have constructors for them to avoid the issue entirely?

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

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.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

@foolip would be great if you could check those changes, and the corresponding testsuite fixes. Happy? :)

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

You still need to take a second argument, probably just EventInit, or it won't be possible to set bubbles and cancelable.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Also a few follow-up issues:
#13
#14

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

IDL is updated to have a second optional EventInit argument.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Thanks!

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

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?

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

You want @smaug---- and @ehsan to r+ this from our side.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Thanks, I patiently await their judgment :)

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

Not sure what I should comment about here.
What is in the current spec looks wrong to me, it is not consistent with the rest of the events.
There should be dictionary for ClipboardEvent.

(off topict, it is partially because of consistency in Event interfaces which lets Gecko to autogenerate C++ implementations from most of the *Event interfaces)

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

There should be dictionary for ClipboardEvent

If so, what should be its members? It's not possible to create a DataTransfer object.

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

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).
But I'd make dictionary to have
DataTransfer? dataTransfer = null;
and then the event would have also nullable dataTransfer

other option is to use 'required' in the dictionary.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Is this on principle, that because ClipboardEvent.clipboardData exists, so should ClipboardEventInit.clipboardData?

If the only way to get a DataTransfer instance is from a trusted ClipboardEvent or DragEvent, then I don't think there's anything sane that one could do with ClipboardEventInit.clipboardData, but it would be more tests to write.

In any case, DragEvent.dataTransfer and DragEventInit.dataTransfer are nullable, and I agree that's at least better than non-nullable equivalents.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

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.

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

er, I meant clipboardData in my comment, not dataTransfer. And yes, it is on principle that since
clipboardData exists, so should ClipboardEventInit.clipboardData.

(I do assume we'll want to make DataTransfer constructible at some point.)

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

@smaug---- is this OK? https://w3c.github.io/clipboard-apis/#clipboard-event-interfaces

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

The ClipboardEvent.clipboardData member also needs to be nullable now.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

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.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

Nullable stuff seems generally bad unless there's a good case for them.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

In that case ClipboardEventInit.clipboardData should also not be nullable, which would bring us back to the constructor being almost entirely useless, as you couldn't call it unless you already had a DataTransfer instance from another event.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

Which seems fine. NotificationEvent is like that.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Like this?

dictionary ClipboardEventInit : EventInit {
  required DataTransfer clipboardData;
}

That would work for me, since it appears actually being useful isn't a requirement for event constructors :)

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

Yes.

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

that looks fine, although I'd prefer nullable in this case since FF has shipped some version of
ClipboardEventInit , and adding required properties is backwards incompatible change, and
nullable is just fine too.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Presumably making it nullable also isn't risk free, as any existing uses would get an event with a null clipboardData where it was previously always non-null?

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

How could it be non-null for synthetic events?

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

There is no existing use for the constructor as to be defined in the spec.
Gecko has a bit different kind of ctor + init, and it is just that we add 'required' property to the init, any existing use will start to throw.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

How could it be non-null for synthetic events?

Try new ClipboardEvent('paste') in Firefox, it seems to just create a DataTransfer object with no items.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

That seems somewhat magical. Seems like Gecko already breaks some invariants @smaug---- seems concerned with keeping.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Yeah, but I'm assuming @smaug---- will fix it :)

I'd be OK with any of the following:

  1. No constructor at all
  2. A constructor that takes EventInit as its second argument and just creates an empty DataTransfer. (Still too magical?)
  3. An ClipboardEventInit with an optional and nullable clipboardData member.
  4. An ClipboardEventInit with a required and non-nullable clipboardData member.

Take your pick! I'd also like if the same pattern is used for https://www.w3.org/Bugs/Public/show_bug.cgi?id=28984 unless there's some difference between drag and clipboard events I'm missing.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

Well, if you just agree on your IDL polishing (or even do a Github PR) I'll update the spec - OK? ;)

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

@annevk, @smaug----, do you care which of the options in #10 (comment) prevails, or is anything fine at this point?

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

I don't like 2. It seems @smaug---- has a slight preference for 3, which I don't particularly mind.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

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?

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024
  1. sounds ok to me, and wontfixing bug 28984 sounds good too.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Do you really mean 1 as in no ClipboardEvent constructor, but to leave the DragEvent constructor as it is? Is there a difference between these to motivate the difference?

from clipboard-apis.

smaug---- avatar smaug---- commented on July 20, 2024

Er, I mean 3.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Great, so I think the only remaining change is to make ClipboardEvent.clipboardData nullable.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

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

evt = new ClipboardEvent('paste');
evt.clipboardData.setData('text/plain', 'hi');

won't work because evt.clipboardData is null? That will make synthetic paste events pretty useless..

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

That would be option 2 from #10 (comment), which @annevk doesn't like.

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

Synthetic events are generally useless. They're a debugging aid.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

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.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

(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 ..??)

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

@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

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

@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.)

from clipboard-apis.

annevk avatar annevk commented on July 20, 2024

Event object properties are just copied automatically from EventInit?

Yes, per a macro.

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?

There's a couple things wrong with this question, but the main one is that you cannot really say it creates a DataTransfer object out of thin air. It needs to be supplied.

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Ping? @Tanayc is kind of blocked on this issue, what's needed to reach a conclusion here?

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

@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.. :-/

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

I made a minor change to align DragEventInit with ClipboardEventInit in whatwg/html#71. What still needs to change is make ClipboardEvent's clipboardData member nullable.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

@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 :)

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

I agree, with a DataTransfer constructor this wouldn't have had to be nullable. I guess the reason nobody has tried to do that is because the constructor would be a bit complicated, in the end because DataTransferItem can represent either a file or a string. Not insurmountable, though. Maybe file an issue on whatwg/html if you have a syntax in mind?

I commented about a missing ?.

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

I just need to demonstrate my lacking understanding of WebIDL :/
Missing ? fixed in 2897399 which was the only variant that gave me the expected output so I assume it's correct :p

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Wait, why did you drop = null again? Without that you'd have to define in prose what the default value is :)

from clipboard-apis.

hallvors avatar hallvors commented on July 20, 2024

Because the output (as in the spec text you read) when I add the = null becomes

8.2 Attributes
    null of type DataTransfer? clipboardData =, readonly 

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..

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

Sorry, I didn't look closely and thought I was looking at a ClipboardEventInit member... What's currently live at https://w3c.github.io/clipboard-apis/ looks good, thanks for fixing!

from clipboard-apis.

garykac avatar garykac commented on July 20, 2024

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

from clipboard-apis.

foolip avatar foolip commented on July 20, 2024

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 new ClipboardEvent('foo') to throw an exception.

from clipboard-apis.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.