[Safari] [Development tracking] backups on Safari (make them work)

[chrisaljoudi]

@gorhill backups use vAPI.download which is defined in vapi-common.js. The thing is, the function uses vAPI.messaging.channel:

var messager = vAPI.messaging.channel('_download');

That's only defined in vapi-client.js. However, vapi-background.js seems to overwrite vAPI.messaging altogether and does not define channel() for it.

How/why does vAPI.download work at all in this case?

The problem is, it does not work on Safari — and the error is exactly that vAPI.messaging.channel is undefined (which, I guess, makes sense).

Is there something obvious I'm missing? :)

[gorhill]

vapi-client.js is for auxiliary pages, not for the background process.

[gorhill]

So if I understand, on Safari, the auxiliary pages see everything in the background process?

[chrisaljoudi]

@gorhill hmm, nope, that's not/shouldn't be the case. Let me look into this for a bit more.

[chrisaljoudi]

@gorhill backupUserData is handled by messaging.js, which seems to be in background.html:

<script src="js/vapi-common.js"></script>
<script src="js/vapi-background.js"></script>
...
<script src="js/messaging.js"></script>

Given this, how's vAPI.download supposed to see vAPI.messaging.channel?

[gorhill]

vapi-common.js is included in the background process, and optionally in auxiliary pages.

There is always a vAPI.messaging when vapi-background.js or vapi-client.js is included.

[chrisaljoudi]

@gorhill I understand that, but clicking "Backup User Data" sends backupUserData over the settings.js channel, right?

That seems to be handled by messaging.js (in the background process), and in that context, vapi-client.js isn't included — which means vAPI.messaging.channel isn't defined.

[chrisaljoudi]

@gorhill

There is always a vAPI.messaging when vapi-background.js or vapi-client.js is included.

Yep! vAPI.messaging is there, but vAPI.messaging.channel is not when you just have vapi-background.js. That simply does not define vAPI.messaging.channel.

[gorhill]

Wait, I am confused, looking into this..

[gorhill]

So this code path is not executed on Chromium, and I am guessing neither for Firefox. Looks like this was meant to be fallback code for when a browser does not support the download property on a tags. So unmaintained/forgotten code?

[gorhill]

Ok.. so my change in d8b949e broke backing up for Safari?

The download operation used to be in the auxiliary page, now it's in the background process.

[chrisaljoudi]

@gorhill I guess, yeah. Changing the code in vapi-common.js to use broadcastof the gotoURL instead of a channel should solve the issue. Does that sound good to you?

Edit: no, it won't. Never mind.

[gorhill]

I think this should be more something like if channel does not exist, call vAPI.tabs.open() directly -- this is what gotoURL is essentially. There are other auxiliary pages still using vAPI.download.

[gorhill]

It might make sense to have a property which tell whether the current scope is the background process or an auxiliary page, this would be more easy to understand the code.

[chrisaljoudi]

@gorhill yes, that would work, but I think the problem is that mixed types of pages (both background and auxiliary) are using vAPI.download.

Edit:

It might make sense to have a property which tell whether the current scope is the background process or an auxiliary page

Perfect.

[chrisaljoudi]

@gorhill 0379762 should've fixed this. I applied the same changes across Firefox and Chrome for:

• consistency
• really old versions that don't support the download attribute

[gorhill]

Not being able to backup feels like kind of a big deal for Safari users... You think we should isolate the fix and apply it to 0.9.0.0 and create a 0.9.0.1 release just for Safari users?

[chrisaljoudi]

@gorhill yes, I'm surprised it wasn't reported.

Hmm, that might cause problems unless we also distribute the fixes for Safari storage (4720ecd and 10f656f), which are pretty major.

At that point, that includes almost everything for Safari.

[gorhill]

Major changes require more testing. Can Safari users install the dev version? I was about to release a new one.

[chrisaljoudi]

@gorhill

Can Safari users install the dev version?

Nope. :(

The way auto-update works for Safari, there would have to be a different update manifest setup, which is not done (yet). Should I do that?

[chrisaljoudi]

@gorhill Also, right now, the backed-up file doesn't consider the filename in Safari (defaults to Unknown). I'm working on that right now.

[chrisaljoudi]

@gorhill bad news: Safari simply doesn't have a way to control the filename from JavaScript. Users will have to do ⌘ + s and then choose a filename if they wish.

I know this is not as good because of the timestamped filenames, but it's what Safari is for now.

The only way to achieve the same functionality (auto-download and a generated filename) would require sending the backup to a server and back. While a server is available, I think that's really iffy privacy-wise.

[chrisaljoudi]

@gorhill what do you think about the problem I mentioned above? Do you think we'll have to live with the limitation for now (given that sending the backup data to a server and back is iffy privacy-wise)?

[gorhill]

Do you think we'll have to live with the limitation for now

Yes.

Sending users' private information on the wire is out of question. Eventually Safari will support the download property I expect.