fix loading of less imports on win, linux and mac

[zaggino]

maybe the if should be more sophisticated, but this fixes #7513 tested on both win & linux ... need to test on mac too to be 100% sure

[peterflynn]

I think the issue here is that, somehow, the path you're getting passed is sometimes a URL and sometimes an OS path. Each of these APIs works correctly across all platforms when given the type of string it's designed to be used for, but since the type of string varies by platform we need to call both. Might want to tweak the comment a bit -- as is it sort of suggests one or both of these APIs is unreliable.

[zaggino]

fixed as suggested @peterflynn
i still need to test this at home on mac to be 100% sure

[redmunds]

@zaggino I don't like the idea of this solution because I think it sets a bad precedent -- pretty soon we'll have to do this everywhere.

The FileSystem.isAbsolutePath() check does not seem to be safe. All it does is check to see if first char is a forward slash (fullPath[0] === "/") which will cause problems if someone passes in a web site root-relative path of something like "/styles/my.less".

What code is failing? Maybe that code should convert the local file path to a url instead.

cc @peterflynn

[zaggino]

Ah, you're right this is a bad solution ... here's the problematic code (line 140):

function getModuleUrl(module, path) {
        var url = encodeURI(getModulePath(module, path));

        // On Windows, $.get() fails if the url is a full pathname. To work around this,
        // prepend "file:///". On the Mac, $.get() works fine if the url is a full pathname,
        // but *doesn't* work if it is prepended with "file://". Go figure.
        // However, the prefix "file://localhost" does work.
        if (brackets.platform === "win" && url.indexOf(":") !== -1) {
            url = "file:///" + url;
        }

        return url;
    }
``

[peterflynn]

@redmunds The FileSystem APIs all only work with paths -- if someone passes in a URL instead, they will all fail. I don't think we should change this one API to accept URLs when none of the other related APIs do.

Where is the path originating from in this case? It seems like it's part of the Require module object. If Require has a feature/bug where it sometimes passes file://... URLs and sometimes passes absolute Linux/Mac system paths, then it seems appropriate to me to place a workaround specific to that issue localized in this one bit of code that actually interacts with those Require APIs.

[zaggino]

This results in url being different for win vs others so different check is also needed depending on the platform...

[peterflynn]

Oops, @zaggino sorry, your replies didn't show up until I reloaded the page. So tt seems like our own getModuleUrl() fully explains the funkiness both cases -- not a Require bug.

In the loadFile() case we could just do the absolute check before converting getModulePath() to a URL... but in the loadStyleSheet() -> parseLessCode() case we're going off of the URL fed into $.get() (this.url), which has to have the funkiness... so it does seem like the cleanest solution overall is to just have an absolute-checking utility that handles both cases...

[redmunds]

@peterflynn

I don't think we should change this one API...

I wasn't suggesting that API should be changed, just not used here.

[zaggino]

I changed the check a bit according to getModuleUrl() and fixed the comment, should be much more clear now.

[redmunds]

Done with review. Looks good -- just need to update comments.

[zaggino]

Updated + Squashed old commits

[redmunds]

Looks good. Merging.