make html5 behavior of onDropFile same as native

[loudoweb]

onDropFile is dispatched for each files on native with the full path of the file as a parameter. But in html5 it is only dispatched once with a FileList as a parameter which is an array. Also this array doesn't give the full path, it is mandatory to call js.html.URL.createObjectURL(file) to get the full path.

So this commit allows to make the behavior across targets consistent. Now onDropFile in the html5 target is dispatched for each files and the full path is passed as a parameter like in the native target.

[player-03]

Interesting. Looks like it's a trade-off between consistency and functionality. Consistency should win out, of course, but I'm worried it isn't as simple as this. (Disclaimer before I go any further: I've never actually made an application using onDropFile.)

It's good to get the file URL, but most app aren't after that alone; they're after the file contents. On most targets, you'd use the URL to call sys.io.File.getContent() (assuming a text file). But since HTML5 doesn't have access to the sys package, you typically need to use js.html.File.text(). So even with this commit, the programmer's code remains inconsistent. They still need an #if js block of code, and what's more, that block of code has to perform the extra step of reconstructing js.html.File from the URL.

I think this is what Joshua was getting at with the TODO. The ideal solution should include some kind of wrapper for js.html.File, allowing the programmer to write the exact same code regardless of target. I guess we'd rewrite sys.io.File? getContent() would construct a js.html.File object from the URL, load the data, and block until complete. (Feels inefficient not to reuse the existing js.html.File objects, but those aren't available.)

[loudoweb]

I understand but my cross-platform app works better like this. I can get the image dropped in my browser with BitmapData.loadFromFile(path). So I don't have #if in my code. I haven't tried with different type of files. I'll check with text and sounds tomorrow.

[loudoweb]

So it seems that you get a point. On native you have the full URL with extension. And with my PR you have this kind of url: blob:http://127.0.0.1//fdsfkuhdsikufhdsiuyfgh so without extension so can't know what kind of file it is. My code worked with images but I only tried with images. If you drop a text or sound, you can't know that is this kind of file.

So we need a wrapper like you suggest, this is my proposition: Instead of returning a String, it could return this: {extension: "png", path: "http://..."}. So the user knows what the file is on html5. On native, it is better than having just the path alone, the user would not have to get the extension by himself because we'll provide it to him. Note: I prefer to give the extension to the user instead of mime type because user can have its own file extension and I'm unsure on how to treat unknown files. Plus I don't know if we can get the mime type on native.

I'll try to update my PR.

EDIT I may add name: "image.png" in the output format because the url provided in html5 doesn't use the original file name so there is a lost of information.

[player-03]

Any chance the wrapper could be an abstract that implicitly casts to string? This would make it easier to update existing code.

[loudoweb]

I can try but I'm a noob in abstract.

[loudoweb]

@player-03 something like that?

https://try.haxe.org/#9299BBA6

Sadly I don't know how to avoid var t = p.toObject(); for the new way but old way use implicit cast

[loudoweb]

Oh I think I got it:

https://try.haxe.org/#47279a34

class Test {
  static function main() {
    
    
    var p:ADropFile = new ADropFile({path:"http://", type: "png", name: "plop"});
    
  	onOldDropFile(p);
    
    onDropFile(p);
    
  }
  
  static function onOldDropFile(p:String){
		trace(p);
  }
  
  static function onDropFile(p:TDropFile){
    trace(p.path, p.name, p.type);
  }
  
  
  
 
}
  
abstract ADropFile(TDropFile) {
  
  
  inline public function new(data:TDropFile) {
    this = data;
  }


  @:to
  public function toString() {
    return this.path;
  }
  
  @:to
  public function toObject():TDropFile {
    return this;
  }
}

typedef TDropFile = {path:String, type:String, name:String};

[loudoweb]

Sadly I can't use an abstract in a lime event. It just doesn't work. So I propose to use the TDropFile typedef. I can still use a conditional to keep the old method but I think it's better to make people move forward, it should be simple to update the code (=> new lime will prompt an error with wrong parameter => just change the parameter and that's it).

[player-03]

Sadly I can't use an abstract in a lime event. It just doesn't work.

Are you sure? It works perfectly for me, every time. I tested with my own abstract, with your implementation, and with an updated version I made.

Speaking of which, my updated abstract is a little more concise, includes @:forward, and automatically parses URLs. The latter could help backwards compatibility. Here's how it works with Lime's events:

var limeEvent = new Event<ADropFile -> Void>();
limeEvent.add(function(dropFile:ADropFile) {
	trace(dropFile.name);
	trace(dropFile.type);
	trace((dropFile:String));
});
limeEvent.dispatch("http://www.example.com/plop.png");

Expected output: plop, png, and http://www.example.com/plop.png. Does it work differently for you?

[maitag]

i've wrote some drag&drop-limesample sometimes ago: https://github.com/maitag/dragdrop Can i reduce the html-branched code there now ... or maybe useful also to put into lime-samples for testing?

[player-03]

Yes, you'd be able to reduce the branching code, but the question is how much. There are actually multiple things that force branching.

First: you're adding a listener in an HTML5 app and overriding a function otherwise. Instead, you could use Window.onDropFile, since it's available on all platforms.

The second is that Window.onDropFile is lying to you. It claims to be an Event<String->Void>, meaning the listener will receive a string. On most targets, this is true, but on HTML5 you get the event.dataTransfer.files array. (Seen here in your code.) A useful array, but it isn't what you were promised, and it isn't consistent. Thus your listener function needs to handle two entirely different data types, depending on platform.

This pull request (as submitted 11 days ago, with no further changes) would address the second issue. Your listener would always receive a string, containing the filename/URL. That's all Lime promises to do: deliver a string and allow you to handle it. We might be able to stop there.

But you would immediately run into a third issue: there's no easy cross-platform way to use the string you were given. Some platforms have access to sys.io.File, but not HTML5. HTML5 has FileReader, but nothing else does. Lime is all about making cross-platform code easier, so this is really ought to be our responsibility.

The abstract we're discussing would do some parsing and provide some convenience functions, but currently does nothing to solve this issue. It's just syntax sugar wrapping the same old filename/URL, and different platforms still require different code to load the file contents. But the abstract does have the ability to store additional data, and we could include an Input object. On HTML5 targets, this Input would wrap a FileReader, and on system targets it'd be a FileInput. Then you could call readString() or readBytes() no matter what platform you're on.

[player-03]

I have three concerns at this point.

1. The abstract will break existing code. If your function expects a string, it won't serve as an onDropFile listener. It's an easy update but still makes this a breaking change, which means this pull request would have to wait until a new major version of Lime comes along. (For various reasons, it's impractical to work around this on Lime's end.)
2. Input always blocks, while HTML5 code usually loads files asynchronously. HTML5 programmers might feel like this is a step back. (It's possible to work around this on Lime's end, but would require effort.)
3. Someone else might have already solved the file loading problem, and if so there's a good chance they did it better. We might be reinventing the wheel here.

I'm leaning towards a different solution. Don't wrap the string. Don't modify onDropFile. Pass the string, as promised, and provide a cross-platform way to make use of it. Maybe we'd re-implement sys.io.File for HTML5, or maybe we can find an existing library and point users towards that. Users could use our suggestion or something else (such as BitmapData.loadFromFile() if they know for sure it's an image, or FileLoader if they only target HTML5).

This would still technically be a breaking change, but it's fixing an obvious error, so I feel like it gets a pass.

[loudoweb]

Sorry I'm currently in holyday so I'll answer when I'll be back.

[maitag]

Thx to response... (first i did made that old sample compatible to lime 7.9.0 now! ;) The thing is also that i am using "whatformat" haxelib there to let only load the file if it have an expected fileformat. Because file-ending can be not the same as what is into magic header the goal was to stop loading immediately after reading the file-header-data and that diffs into behavior on html and native targets.

[player-03]

Potential solution: https://github.com/ShaharMS/lime/pull/1