1178609 - `join` in loader.js does not work with resource:///

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[James Long (:jlongster)]

I am creating a main module like this:

  let mainModule = loader.Module(
    "resource:///modules/devtools/debugger/",
    "resource:///modules/devtools/debugger/main.js"
  );

However, there is a bug inside loader that doesn't work with `resource` paths with a triple-slash. This is how you loader browser frontend files, so I need to do this.

`join` already looks pretty busted. In the comments it says that the `normalize` function it uses takes out a slash, and we need to add it back in. It looks like in my case, it takes out *two* slashes.

I have no idea what the elegant solution is. This made it work for me:

function join (...paths) {
  let tripleResource = false;
  if(paths[0].indexOf('resource:///') === 0) {
    tripleResource = true;
  }
  let resolved = normalize(pathJoin(...paths))
  // OS.File `normalize` strips out the second slash in
  // `resource://` or `chrome://`, and third slash in
  // `file:///`, so we work around this
  resolved = resolved.replace(/^resource\:\/([^\/])/,
                              tripleResource ? 'resource:///$1' : 'resource://$1');
  resolved = resolved.replace(/^file\:\/([^\/])/, 'file:///$1');
  resolved = resolved.replace(/^chrome\:\/([^\/])/, 'chrome://$1');
  return resolved;
}

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to James Long (:jlongster) from comment #0)

> `join` already looks pretty busted. In the comments it says that the
> `normalize` function it uses takes out a slash, and we need to add it back
> in. It looks like in my case, it takes out *two* slashes.

It takes two slashes also from file:///, already. Basically, it reduces to one slash, no matter what – and not sure why. 

> I have no idea what the elegant solution is. This made it work for me:

We could probably do something like that:

function join (...paths) {
  let joined = pathJoin(...paths);
  let resolved = normalize(joined);

  // OS.File `normalize` strips out the second slash in
  // `resource://` or `chrome://`, and third slash in
  // `file:///`, so we work around this
  let re = /^(resource|file|chrome)(\:\/{1,3})([^\/])/;
  let matches = joined.match(re);

  return resolved.replace(re, (...args) => args[1] + matches[2] + args[3]);
}

We basically put back the slashes originally given, for such schemes.

Comment 2

[James Long (:jlongster)]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #1)

> We basically put back the slashes originally given, for such schemes.

Sorry I didn't reply to this! The leak issue has been hugely distracting... Yeah, that approach sounds great!

Comment 3

[James Long (:jlongster)]

Attachment: 1178609.patch

This is exactly your code, but take a look anyway :p I know you'll probably want tests, but I still can't get any of the tests to run. My patches are really tiny though and it shouldn't be hard to write a quick test, but I'll need your help with that if that's ok?

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: https://github.com/mozilla/addon-sdk/pull/2022

James, I took the code and add the unit test, let me if it's OK. I created a PR directly on github, to merge with SDK master repo.

Comment 5

[James Long (:jlongster)]

Comment on attachment 8632792 [details] [review]
https://github.com/mozilla/addon-sdk/pull/2022

cool thanks!

Comment 6

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b669babc413f8e81895f37362ec1b37124cf6275
Bug 1178609 - `join` in loader.js does not work with resource:///

https://github.com/mozilla/addon-sdk/commit/d046f17c75a25e3a5f5296bab1bd7fce0533ac59
Merge pull request #2022 from ZER0/join/1178609

fix Bug 1178609 - `join` in loader.js does not work with resource:///

Comment 7

[James Long (:jlongster)]

Leaving open until it lands on m-c, which I'll do soon

Comment 8

[James Long (:jlongster)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3250f4708ba9

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b88ff909d103

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b88ff909d103