Add import userscript from file #2804

Sxderp · 2018-01-14T19:58:18Z

Addresses #2612.

Not as nice as being able to navigate to the file but the the baseline result is the same. Unfortunately due to Mozilla issues with the panel a new tab has to be opened. When the relevant bug is fixed the import code should be copy/pastable directly into the Monkey Menu (necessary elements are in place).

I also have another branch that deals with import/export of the entire database, but that's somewhat reliant on the value-store PR (well, it could be done without the PR, but it makes things a bit simpler).

arantius · 2018-01-26T15:50:55Z

Extract this zip file: icon.zip

Pick import, browse to that .user.js. Console:

at installFromSource(), db fail: TypeError: alert.png is not a valid URL.
Stack trace:
safeURL@moz-extension://ec66ea3f-5475-415e-b7af-7f37a4899dfe/src/parse-user-script.js:33:12
window.parseUserScript@moz-extension://ec66ea3f-5475-415e-b7af-7f37a4899dfe/src/parse-user-script.js:124:25
installFromSource/</<@moz-extension://ec66ea3f-5475-415e-b7af-7f37a4899dfe/src/bg/user-script-registry.js:72:23
...

After a couple seconds, a broken editor with no script pops up. Nothing was installed. I believe this will happen for any relative resource reference, this one is an @icon, but @resource or @require would do it too.

Sxderp · 2018-01-26T20:24:13Z

After a couple seconds, a broken editor with no script pops up. Nothing was installed. I believe this will happen for any relative resource reference, this one is an @icon, but @resource or @require would do it too.

How would a WebExtension have access to these files in order to import them? I suppose you could import the entire .zip. But this was just for a single userscript. And of course, if you have relative paths (no scheme) in a remote script, you're going to hit errors too.

Sxderp · 2018-01-26T20:25:35Z

Alternatively, are you just pointing out the point that the editor pops open and shouldn't? That is fixable. As for actually getting that script to install, I'm not sure.

arantius · 2018-01-26T20:56:39Z

And of course, if you have relative paths (no scheme) in a remote script, you're going to hit errors too.

Try e.g. https://arantius.com/misc/greasemonkey/protect-textarea.user.js which contains the line // @icon img/textarea-padlock.png and works perfectly.

Alternatively, are you just pointing out the point that the editor pops open and shouldn't?

More generally that the error condition is not handled (because it's not even detected?). And predicting the users that will see exactly the feature they want, except it doesn't work in the case they want to use it for.

Sxderp · 2018-01-26T21:05:59Z

Try e.g. ..

Ah, okay. I must have missed that functionality. Cool.

More generally that the error condition is not handled (because it's not even detected?).

Detecting the error is doable. Of course getting the resources from the disk is probably a no-go. However, getting them remotely may be possible. Of course the implicit downloadUrl set due to navigation / remote install won't be applied from a local install, but it might be possible to modify parse-user-script.js to use any explicit @downloadUrl metatag when resolving resource locations.

Sxderp · 2018-01-28T19:48:43Z

Added an error if the URL cannot be resolved to a full path (with notification). If @downloadURL is present then that will be used as the base. Of course, the current installation flow for installFromSource does not download remote data. So nothing is fetched. However, that can be resolved in a later PR I think (I'm working on one, but it still needs work).

arantius · 2018-02-02T17:49:16Z

src/browser/monkey-menu.js

+    event.stopPropagation();
+    event.stopImmediatePropagation();
+  }
+  document.getElementById('import-script-file')


The element with this ID is hidden by CSS, so is not clickable, so this code doesn't work.

It is indeed clickable. Otherwise the installation page would never open.

When I check out this branch I see:

The <a href="#import-user-script" ...> and its text, but no <input>. Clicking the (visible) anchor does nothing. The CSS sets <input> opacity to zero so maybe it's supposed to be overlaid (towards what end?), but that's broken. If remove the opacity setting:

The position is off? Like I said above, the half-this-half-that implementation is suboptimal. I was indeed not clicking on it/it was invisible.

Ah, must have to do with the recent fixes to the MM styling. When I tested it just happen to be in the right location to work.

(towards what end?),

I thought this was obvious. You can't style input fields. So either you hide it and overlay it on text you would like to display or you're stuck with a "browse" button with absolutely no context. Now, I can grant you that at this moment in time the file picker is unnecessary, since they don't work (RIP Mozilla please). But down the line, unless you want to stick with the poor UI of a file input, the overlay is pretty much required.

Either way, I'll remove the boilerplate as it's, as you said, not needed at the time.

arantius · 2018-02-02T17:53:29Z

src/parse-user-script.js

@@ -77,6 +118,9 @@ window.parseUserScript = function(content, url, failIfMissing) {
    case 'noframes':
      details.noFrames = true;
      break;
+    case 'downloadURL':


It was a very intentional decision to not handle downloadURL, updateURL, etc. in GM4. I won't merge code that tries to add them again.

This causes a situation that breaks the principle of least astonishment. I might choose to save a script to my own disk/server, perhaps to isolate it from the original author. But then by accident through one little setting it downloads/updates to some other remote resource that someone else controls? Do not want.

What do you propose then? We can't get the resources from disk without the user explicitly asking to open a file. As far as I'm aware we can't even open a 'please select next file' picker. Even if we could, asking the user to select every resource is pretty bad UX. Granted, the resources aren't downloaded due to current limitations of installFromSource but that should eventually be fixed.

So, should the script be imported without the external resources? (Yes, that's what happens now, but at least with the current PR the URLs are available and can be used to fetch resources).

Everything could be .zip but that could be pretty extreme for just a single script.

Alternatively, I could make it so that the original request takes precedence over the @downloadURL, that way if you're hosting it somewhere then resources will be fetched from your host, but if you're installing it by just importing the script it can still fetch remote resources using @downloadURL.

Or do we just not support relative paths? Which will probably break a lot of compatibility, no?

I should also say that some of the above also applies to the full DB import / export. Of course if you're importing an archive created by GM then you have access to everything as it was. However, if you import an archive exported from TM or VM then the only 'bits' that GM understands is the raw script file itself and the value-store (if importing from TM). The previously set, internal, downloadUrl will be unavailable (non-existant) and we're stuck with not being able to fetch resources.

Not all scripts specify @downloadURL, so users would be left with sometimes working sometimes not, even with this.

It would make plenty sense to me to allow: 1) .user.js import, if no deps (or all fully-specified and currently downloadable remote refs) and also 2) .zip import where there's exactly one .user.js and relative-only references to deps otherwise in the zip.

There's also some way to select a directory with a file browser? I don't know what access you get to files in that directory though.

arantius · 2018-02-02T17:54:58Z

src/browser/monkey-menu.js

@@ -153,10 +159,26 @@ function onHashChange(event) {
 }


+// Import a new user script
+// TODO: Add onImportUserScript to monkey menu when file picker bugs are fixed


I find it counterproductive and confusing to have this whole thing written half in one style and half in another. I'm also not keen on the awkward wording in the UI.

Write it so that it works well, now, complete. If and only if in the (distant, if ever) future something different becomes possible, then and only then write code to do that.

Sxderp · 2018-02-02T23:50:01Z

I'm going to close this and open a new one, branching from the initial commit. The new PR will focus on 1) from #2804 (comment). Adding 2) or 3) I think can be done on a later date since they focus on what's probably more of an edge case.

I'm also going to work on my import / export DB branch to follow the same ruleset as above. If an importing script doesn't have fully qualified resource paths then it won't be imported (unless the archive contains the GM 'bits').

Sxderp force-pushed the import-script-from-file branch 5 times, most recently from 50c0f3e to 39efdc0 Compare January 14, 2018 20:58

arantius added this to the 4.3 milestone Jan 15, 2018

Sxderp force-pushed the import-script-from-file branch 7 times, most recently from be1b004 to a0a1058 Compare January 22, 2018 20:14

Sxderp force-pushed the import-script-from-file branch 3 times, most recently from 0cf4e56 to 7723b60 Compare January 28, 2018 19:46

Sxderp force-pushed the import-script-from-file branch 3 times, most recently from fa23eb0 to c1f8227 Compare February 1, 2018 23:46

Sxderp added 5 commits February 1, 2018 18:46

Add import userscript from file

b47dc1f

Create custom errors for parse-user-script

b05f314

Handle explicit @downloadURL metavar

0d5a548

Safely handle ParseErrors in installFromSource

33d9694

Adjust ParseError notification title for locales

c6c4ffa

Add locales to script import page

c1f8227

arantius removed this from the 4.3 milestone Feb 2, 2018

arantius requested changes Feb 2, 2018

View reviewed changes

Sxderp closed this Feb 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add import userscript from file #2804

Add import userscript from file #2804

Sxderp commented Jan 14, 2018

arantius commented Jan 26, 2018

Sxderp commented Jan 26, 2018

Sxderp commented Jan 26, 2018

arantius commented Jan 26, 2018

Sxderp commented Jan 26, 2018

Sxderp commented Jan 28, 2018

arantius Feb 2, 2018

Sxderp Feb 2, 2018

arantius Feb 2, 2018 •

edited

Sxderp Feb 2, 2018

arantius Feb 2, 2018

Sxderp Feb 2, 2018

Sxderp Feb 2, 2018

arantius Feb 2, 2018

arantius Feb 2, 2018

Sxderp commented Feb 2, 2018

Add import userscript from file #2804

Add import userscript from file #2804

Conversation

Sxderp commented Jan 14, 2018

arantius commented Jan 26, 2018

Sxderp commented Jan 26, 2018

Sxderp commented Jan 26, 2018

arantius commented Jan 26, 2018

Sxderp commented Jan 26, 2018

Sxderp commented Jan 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arantius Feb 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sxderp commented Feb 2, 2018

arantius Feb 2, 2018 •

edited