P5.js: p5.dom.js drop function reads .json files as base64 encoded with application/json type instead of a clear text/json type

[n-peugnet]

Nature of issue?

• [ ] Found a bug
• [x] Existing feature enhancement
• [ ] New feature request

Most appropriate sub-area of p5.js?

• [ ] Color
• [ ] Core/Environment/Rendering
• [x] Data
• [ ] Events
• [ ] Image
• [ ] IO
• [ ] Math
• [ ] Typography
• [ ] Utilities
• [ ] WebGL
• [x] Other (specify if possible)
  p5.File of p5.dom.js

Details about the bug:

• p5.js version: 0.6.0
• Web browser and version: Chrome 70.0
• Operating System: Windows 10

Feature enhancement details:

Using the drop function with a .json file gives me a base64 encoded data, I guess that this comes from the fact that json files are read as application/json type files instead of text/json.

.json files should be read like a plain text file so that it is possible to easily get it's content. Right now I have to change the extension of my file into .txt to get it's content through p5.File.data which is not really convenient

[limzykenneth]

.json files will always be read by the browser as application/json as that is its valid MIME type, text/json is not an accepted MIME type that is used to identify JSON files and I would strongly suggest not attempting to coerce it to.

Instead we can improve the type inference here to identify application/json MIME type and load that in as text and not as Blob.

[n-peugnet]

Yes I just learned that this is the valid MIME type for .json files.
The solution you are proposing is probably the best, but I can't think of a better way than having an array containing every imaginable text MIME types.

Currently I am solving my problem like so:

fetch(file.data)
    .then(res => res.blob())
    .then(blob => {
        var reader = new FileReader();
        reader.onload = function () {
            var json = reader.result;
            doSomethingWithJson(json);
        }
        reader.readAsText(blob);
    });

[Spongman]

maybe it's worth doing something similar to what httpDo does? .json being returned as parsed objects.

[limzykenneth]

We might come across other files types in the future where the current checks breaks down but for now we can just deal with them on a case by case basis and not try to deal with everything at once.

@Spongman I think that is a good idea. Most people probably won't want to load in JSON without parsing it I guess.

[akshay-99]

Hey @limzykenneth this is quite an old issue but can I try this?
Also maybe we can do the same thing as httpDo for xml data as well ?

[limzykenneth]

@akshay-99 Sure, go ahead! JSON should be parsed into an object while XML should be parsed into a p5.XML object. Let us know if you need help.

[akshay-99]

@limzykenneth So I just need a bit of a clarification.

Currently, the function returns a p5.File to the callback. Within this the data field contains either the text of the file or the base64 string. I am presuming that I should continue using p5.File for json and xml data as well.

My instinct was to replace the contents of data to an object or p5.XML respectively. But the reference document states that data is "URL string containing image data".

So should I go ahead and replace the contents of the data field for json and xml files? Or should I add an additional field in p5.File that will contain the parsed objects

[limzykenneth]

The behaviour should more or less stay the same, ie. the loaded JSON data or XML data should be placed in the data field of the p5.File object as you proposed. The bit that needs to be changed in the code is where type checking occurs, and the idea is to support JSON and XML type and see where the parsing of the loaded data can occur before passing the p5.File object to the callback.

The documentation of p5.File should be updated as well to something more generic as the data field can contain a variety of data.

Although on second thought I wonder if the data really should be parsed into objects rather than staying as strings which the user can parse themselves? 🤔

[akshay-99]

Although on second thought I wonder if the data really should be parsed into objects rather than staying as strings which the user can parse themselves? :thinking:

Yeah and hence I asked about the second option of keeping the data field as it is, and adding another field that contains the parsed stuff. But I guess that's not really a useful feature to have as users can simply parse textual data themselves in just one extra step.
So for now I would just go ahead and do what the issue originally asked for. Just making sure json files dont give us base64 data :sweat_smile:

[limzykenneth]

Do go ahead with that change but we need to finalize whether to parse the object or not before we merge otherwise to change it would be a breaking change which cannot be merged after the 1.0 release for some time.

[n-peugnet]

Hi, just to give my opinion as I was the original poster. It is quite a hassle to obtain a clear string from the base64 one, but it's really easy to convert a json string into a js object. So for the json mime I would be ok with a clear json string. Xml on the other hand is more difficult to parse in js as far as I know and you seem to already have a p5.xml object so for this one it would make sense to return a parsed xml object in data.

[akshay-99]

I have added a simple fix for json. @limzykenneth please take a look.
If needed I can parse XML by adding this code from httpDo here

const parser = new DOMParser();
const xml = parser.parseFromString(text, 'text/xml');
return new p5.XML(xml.documentElement);

Whether JSON and XML should be parsed is still up for discussion. But I am now of the belief that it should be left to the users to parse the data if they want. :thinking:

[limzykenneth]

Ideally the behaviour of passing in either JSON or XML would be the same, ie if parsing XML, JSON should be parsed as well, vice versa. I'll comment on a couple details on the PR itself.

The reason I'm rethinking whether to parse the objects or not is based around the use case of drop(), since it is a function defined on p5.Element it could very well be that the file is dropped onto a div or textarea field, which most users will probably want something along the lines of populating the element with the text content of the file.

Yet it is also true that serialization of objects (expecially p5.XML) is much easier than parsing objects. Unless parsing XML can be made easier somehow, parsing here still sounds plausible.

@lmccart @stalgiag @outofambit Do you have any idea?

[outofambit]

deally the behaviour of passing in either JSON or XML would be the same, ie if parsing XML, JSON should be parsed as well, vice versa.

@limzykenneth i agree! i think i'm in favor of parsing XML and JSON by default in drop(). since these are "data" text files (as opposed to .rtf or .txt), which i imagine are more useful for uploading a config or bunch of settings instead of putting text in the DOM. (and as you point out, serializing is much easier for both if that's what the sketch needs, so it feels like a safe option.)

[akshay-99]

@limzykenneth @outofambit yes that seems reasonable. If you say so I will go ahead and implement it.
But I had another question regarding handling files which are improper JSON or improper XML. DOMParser gives a document containing the error while JSON.parse throws an error. So should I do something to make this uniform. Like making both give an error or both give the unparsed text?

[limzykenneth]

@akshay-99 You can go ahead with the implementation of parsing the contents.

I think making the behaviour uniform might add too much complexity to the code, so I'm thinking of just preserving the existing behaviour which for the case of XML, means to just parse the returned error document from DOMParser and let the user handle it when they come across it. Unless you feel you can make the XML throw a useful error without too much overhead.

They definitely should be erroring instead of returning unparsed text, otherwise it can be hard for users to debug.