1580838 - FileLink WebExtensions API Enhancements and Bugfixes.

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[Diego Casorran [:diegocr]]

Attachment: handle errors loading files, and allow to passthrough File instances.

I am attaching a patch to address the issues highlighted in https://bugzilla.mozilla.org/show_bug.cgi?id=1481052#c64

I.e. it does:

1. Fixes the error handling loading files.

2. Allow FileLink extensions to get a File instance instead of loading the file's contents for them.

Any mentor who would like to check this out? Not sure if that's the correct way to submit patches here.

Comment 1

[Patrick Cloke [:clokep]]

Comment on attachment 9092356 [details] [diff] [review]
handle errors loading files, and allow to passthrough File instances.

I suspect Geoff is the best person to look at this.

Comment 2

[Geoff Lankow (:darktrojan)]

Comment on attachment 9092356 [details] [diff] [review]
handle errors loading files, and allow to passthrough File instances.

This is going to need some more work to get the test and schema updated, but on the whole I'm pretty happy with the idea.

Would you like me to take it from here?

Comment 3

[Diego Casorran [:diegocr]]

Sure, feel free to take over. I do assume that's the faster way to get this shipped with the next TB version. Thanks!

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: 1580838-cloudfile-as-file-1.diff

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9095396 [details] [diff] [review]
1580838-cloudfile-as-file-1.diff

Review of attachment 9095396 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/extensions/parent/ext-cloudFile.js
@@ +18,5 @@
>    let blob = await File.createFromNsIFile(nsifile);
>  
>    return new Promise((resolve, reject) => {
>      let reader = new FileReader();
> +    reader.addEventListener("loadend", ev => {

nit: slightly unusual name. Please use e or event

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b1480206231d
Add option to CloudFile API to get files as File objects instead of ArrayBuffer; r=mkmelin

Comment 7

[Geoff Lankow (:darktrojan)]

Thanks for your patch, Diego!

Comment 8

[Diego Casorran [:diegocr]]

You are welcome, i would also like to leave the door open for possible further enhancements :)

Comment 9

[Magnus Melin [:mkmelin]]

If you find anything you'd like to improve, we're always interested in patches ;)

Comment 10

[Diego Casorran [:diegocr]]

Yeah, that's what i meant, it's very possible that i will submit further patches :)

Comment 11

[Geoff Lankow (:darktrojan)]

Comment on attachment 9095396 [details] [diff] [review]
1580838-cloudfile-as-file-1.diff

I'd like this to be in ESR after some time on beta.

Comment 12

[Jorg K (CEST = GMT+2)]

I'd like this to be in ESR after some time on beta.

What do I make of that? Shipping TB 71 beta now/tomorrow/the day after (26th Oct 2019?). We'll do 68.2.1 maybe in the week after. Is that too early?

Comment 13

[Geoff Lankow (:darktrojan)]

Assuming there are no problems reported I'm okay with it being in 68.2.1.

Comment 14

[Jorg K (CEST = GMT+2)]

TB 68.2.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/bae5514de050c3168cb4bfe36b9c5ebf6acfeeff

This needed rebasing. I ended up landing it without the ExtensionTestUtils.failOnSchemaWarnings() stuff.