New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

absoluteTo() for two single files results in an absolute path #29

Closed
cburgmer opened this Issue Jun 19, 2012 · 8 comments

Comments

Projects
None yet
2 participants
@cburgmer

cburgmer commented Jun 19, 2012

Unclear on what the real usage of absoluteTo() is, I assumed it was similar to Python's urlparse.urljoin.

So for example it will base a filename on top of another relative path:

>>> (new URI("aFile")).absoluteTo("aDir/").toString()
"aDir/aFile"

Especially interesting, when the path passed to absoluteTo() is a path to a file resource, too:

>>> (new URI("aFile")).absoluteTo("aDir/anotherFile").toString()
"aDir/aFile"

However, when applied to two file resources, a slash is added which results in an absolute path:

>>> (new URI("aFile")).absoluteTo("anotherFile").toString()
"/aFile"

I would expect just

"aFile"

as there is no valuable path information in the given string.

I don't know if I am misusing the method, or if this really is a bug. In the latter case I will be happy to provide a fix.

@cburgmer

This comment has been minimized.

Show comment
Hide comment
@cburgmer

cburgmer Jun 19, 2012

I just found that directory() returns a similarly unexpected result:

>>> (new URI("aFile")).directory()
"/"

I'd guess that this might the root of my problem.

cburgmer commented Jun 19, 2012

I just found that directory() returns a similarly unexpected result:

>>> (new URI("aFile")).directory()
"/"

I'd guess that this might the root of my problem.

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jun 20, 2012

Member

absoluteTo() was supposed to "reverse" the effects of relativeTo(). It was never intended to be used as the general purpose directory translation utility you thought it to be.

Examples from the python docs on urljoin:

URI("FAQ.html").absoluteTo("http://www.cwi.nl/%7Eguido/Python.html").toString();
// "http://www.cwi.nl/~guido/FAQ.html"

(doesn't mean we can't pimp this function, though. what exactly do you expect it to do?)

Member

rodneyrehm commented Jun 20, 2012

absoluteTo() was supposed to "reverse" the effects of relativeTo(). It was never intended to be used as the general purpose directory translation utility you thought it to be.

Examples from the python docs on urljoin:

URI("FAQ.html").absoluteTo("http://www.cwi.nl/%7Eguido/Python.html").toString();
// "http://www.cwi.nl/~guido/FAQ.html"

(doesn't mean we can't pimp this function, though. what exactly do you expect it to do?)

@cburgmer

This comment has been minimized.

Show comment
Hide comment
@cburgmer

cburgmer Jun 20, 2012

I need something directly equivalent to the urljoin function. The use case is as follows:

I have an HTML document with given base URI. That links to a CSS file. In this CSS file I reference a backgroundImage. I'd like to know the backgroundImage URL relative to the HTML document. This is basically the URL of the HTML document + URL of the CSS file + URL of the backgroundImage. The HTML document might be relatively sourced and that is the issue here.

I'll have a look at how the function works, and will try to extend it in such a way that it will work with relative paths.

cburgmer commented Jun 20, 2012

I need something directly equivalent to the urljoin function. The use case is as follows:

I have an HTML document with given base URI. That links to a CSS file. In this CSS file I reference a backgroundImage. I'd like to know the backgroundImage URL relative to the HTML document. This is basically the URL of the HTML document + URL of the CSS file + URL of the backgroundImage. The HTML document might be relatively sourced and that is the issue here.

I'll have a look at how the function works, and will try to extend it in such a way that it will work with relative paths.

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jun 20, 2012

Member

Please provide me with a bunch of input/output mappings. I'll construct some tests and try to make the absoluteTo work with that.

(time issues here, if you provide the data I need for testing, I might hack together something tonight or during my flight tomorrow…)

Member

rodneyrehm commented Jun 20, 2012

Please provide me with a bunch of input/output mappings. I'll construct some tests and try to make the absoluteTo work with that.

(time issues here, if you provide the data I need for testing, I might hack together something tonight or during my flight tomorrow…)

@cburgmer

This comment has been minimized.

Show comment
Hide comment
@cburgmer

cburgmer Jun 20, 2012

I've written a wrapper around absoluteTo() which ideally would be included in the function itself. It looks like this:

var joinUrl = function (baseUrl, url) {
        var theUrl = new URI(url);
        if (theUrl.is("relative") && baseUrl.indexOf("/") >= 0) {
            theUrl = theUrl.absoluteTo(baseUrl);
        }
        return theUrl.toString();
    };

Here's the Jasmine tests I've written so far:


        it("should append the url to a directory-only base", function () {
            var url = joinUrl("rel/path/", "the_relative_url");
            expect(url).toEqual("rel/path/the_relative_url")
        });

        it("should append the url to a file base", function () {
            var url = joinUrl("rel/path/something", "the_relative_url");
            expect(url).toEqual("rel/path/the_relative_url")
        });

        it("should merge ../ with a directory-only base", function () {
            var url = joinUrl("rel/path/", "../the_relative_url");
            expect(url).toEqual("rel/the_relative_url")
        });

        it("should just return the url if absolute", function () {
            var url = joinUrl("rel/path/", "/the_relative_url");
            expect(url).toEqual("/the_relative_url")
        });

        it("should combine a url starting with '/' with the host of the base", function () {
            var url = joinUrl("http://example.com/rel/path/", "/the_relative_url");
            expect(url).toEqual("http://example.com/the_relative_url")
        });

        it("should ignore base with an absolute url", function () {
            var url = joinUrl("http://example.com/rel/path/", "http://github.com//the_relative_url");
            expect(url).toEqual("http://github.com//the_relative_url")
        });

        it("should ignore base without directories", function () {
            var url = joinUrl("aFile", "anotherFile");
            expect(url).toEqual("anotherFile");
        });

cburgmer commented Jun 20, 2012

I've written a wrapper around absoluteTo() which ideally would be included in the function itself. It looks like this:

var joinUrl = function (baseUrl, url) {
        var theUrl = new URI(url);
        if (theUrl.is("relative") && baseUrl.indexOf("/") >= 0) {
            theUrl = theUrl.absoluteTo(baseUrl);
        }
        return theUrl.toString();
    };

Here's the Jasmine tests I've written so far:


        it("should append the url to a directory-only base", function () {
            var url = joinUrl("rel/path/", "the_relative_url");
            expect(url).toEqual("rel/path/the_relative_url")
        });

        it("should append the url to a file base", function () {
            var url = joinUrl("rel/path/something", "the_relative_url");
            expect(url).toEqual("rel/path/the_relative_url")
        });

        it("should merge ../ with a directory-only base", function () {
            var url = joinUrl("rel/path/", "../the_relative_url");
            expect(url).toEqual("rel/the_relative_url")
        });

        it("should just return the url if absolute", function () {
            var url = joinUrl("rel/path/", "/the_relative_url");
            expect(url).toEqual("/the_relative_url")
        });

        it("should combine a url starting with '/' with the host of the base", function () {
            var url = joinUrl("http://example.com/rel/path/", "/the_relative_url");
            expect(url).toEqual("http://example.com/the_relative_url")
        });

        it("should ignore base with an absolute url", function () {
            var url = joinUrl("http://example.com/rel/path/", "http://github.com//the_relative_url");
            expect(url).toEqual("http://github.com//the_relative_url")
        });

        it("should ignore base without directories", function () {
            var url = joinUrl("aFile", "anotherFile");
            expect(url).toEqual("anotherFile");
        });
@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jun 23, 2012

Member

Fixed and released with v1.6.2

Member

rodneyrehm commented Jun 23, 2012

Fixed and released with v1.6.2

@rodneyrehm rodneyrehm closed this Jun 23, 2012

@cburgmer

This comment has been minimized.

Show comment
Hide comment
@cburgmer

cburgmer Jun 23, 2012

From the list above that fixes everything with the exception of

    it("should ignore base with an absolute url", function () {
        var url = rasterizeHTML.util.joinUrl("http://example.com/rel/path/", "http://github.com//the_relative_url");
        expect(url).toEqual("http://github.com//the_relative_url")
    });

The idea here is, that you can pass in any url given a base url, but in case of an absolute URL, nothing is changed.

Aside from that, there's a console.log in the change set you probably want to remove.

cburgmer commented Jun 23, 2012

From the list above that fixes everything with the exception of

    it("should ignore base with an absolute url", function () {
        var url = rasterizeHTML.util.joinUrl("http://example.com/rel/path/", "http://github.com//the_relative_url");
        expect(url).toEqual("http://github.com//the_relative_url")
    });

The idea here is, that you can pass in any url given a base url, but in case of an absolute URL, nothing is changed.

Aside from that, there's a console.log in the change set you probably want to remove.

rodneyrehm added a commit that referenced this issue Jun 24, 2012

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jun 24, 2012

Member

Darn. Removed that console.log. Also see the point in returning the original absolute path, rather than forcing it to go relative to the base path. Both are fixed with URI.js v1.6.3

Resolving a resource (e.g. background-image) within a CSS file should be as simple as

var base = '/the/base/path', // from <base ref="…">
    css = 'demo.css', // from <link rel="stylesheet"  href="…">
    image = '../foobar.png'; // from background-image: url(…);

base = URI(base).relativeTo(location.href); 
css = URI(css).relativeTo(base); 
image = URI(image).relativeTo(css);
Member

rodneyrehm commented Jun 24, 2012

Darn. Removed that console.log. Also see the point in returning the original absolute path, rather than forcing it to go relative to the base path. Both are fixed with URI.js v1.6.3

Resolving a resource (e.g. background-image) within a CSS file should be as simple as

var base = '/the/base/path', // from <base ref="…">
    css = 'demo.css', // from <link rel="stylesheet"  href="…">
    image = '../foobar.png'; // from background-image: url(…);

base = URI(base).relativeTo(location.href); 
css = URI(css).relativeTo(base); 
image = URI(image).relativeTo(css);

cburgmer pushed a commit to cburgmer/inlineresources that referenced this issue Mar 30, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment