Users cannot safely redefine url()

[jakob-e]

It seems like the problem is isolated to the case where you pass more than
one argument to @function url($arg1, $arg2: '') { ... }

My tests shows that:

• attr() works with multiple arguments
• omitting the second argument works
• local function variables and global works

Note! When passing the second argument the last character of the first argument is removed

$base-path:'../images/';
$base-attr:'data-';

@function url($src, $path:''){
  @return unquote('url('+$base-path + $path+ $src +')');
}
@function url2($src, $path:''){
  @return unquote('url('+ $base-path + $path+ $src +')');
}
@function attr($arg1, $arg2:''){
  @return unquote('attr('+$base-attr + $arg1 + $arg2 +')');
}

div {
    background: url('image.png');
    background: url('image.png','img/');
    background: url2('image.png','img/');

  &:after {
    content: attr(value);
    content: attr(value, -extra);
    content: url('icon.png');
    content: url('icon.png','gfx/');
    content: url2('icon.png','gfx/');
  }
}

/* Output */
div {
  background: url(../images/image.png);
  background: url(../images/image.pn','img/); /* <= Fails */
  background: url(../images/img/image.png); 
}
div:after {
  content: attr(data-value);
  content: attr(data-value-extra);
  content: url(../images/icon.png);
  content: url(../images/icon.pn','gfx/); /* <= Fails */
  content: url(../images/gfx/icon.png); 
}


/* Expected */
div {
  background: url(../images/image.png);
  background: url(../images/img/image.png);
  background: url(../images/img/image.png);
}
div:after {
  content: attr(data-value);
  content: attr(data-value-extra);
  content: url(../images/icon.png);
  content: url(../images/gfx/icon.png);
  content: url(../images/gfx/icon.png);
}

[jakob-e]

PR added to sass/sass-spec#149 (now containing issue 672 and 674) – Let me know if you want me to create a separate branch and make a new.

[jakob-e]

Extra observation – I'm not sure if it is intentional in LibSass (why I did not make a PR)
Concatenating with null throws an error in SASS 3.4.7 in LibSass 3.0.1 null is ignored.

@function test($arg1, $arg2:null){
  @return $arg1 + $arg2;
}


/* SASS 3.4.7 */
div {
  test-1: test(foo, bar); 
  test-2: test(foo); // Invalid null operation: ""foo" plus null".
}

/* LibSASS 3.0.1
div {
  test-1: foobar;
  test-2: foo; 
}

[xzyfer]

Specs added in sass/sass-spec@28351d9

[xzyfer]

After taking a moment to look into this is turns out this is actually two separate issues.

Issue 1 - url(..)'s signature cannot be redefined

Although you can create an alternate function definition for url(..) the new function signature will be ignored.

@function url($foo/*, this will never matter */) {
     // i can be changed
    @return "doh!";
}

http://sassmeister.com/gist/8d40797d5cf10a1774cf

This is because there is special treatment of url(..) in the parser.

Issue 2 - printed lists are mangled

This is a known issue #639.

Your second parameter isn't ever passed to url(...) due to issue 1. Instead url(...) is getting one parameter $src which is a list. The list is then being interpolated by + causing it to be mangled.

http://sassmeister.com/gist/b6a8b677239082b190ae

---

I've update the issue title to better reflect this as a case of issue 1.

[xzyfer]

This issue has been reintroduced. A bisect shows the offending commit to be 51b0bf0

[am11]

With regard to #532, IMHO the relative paths feature can be implemented as a special case in extensible manner; providing most-common functionality OOTB.

• Add relative-paths option (default: false to remain conformant with ruby-sass).
• When the option is set, on compile compiler will first resolve the value and call a virtual function with default implementation:
  • Sass_Value(char* input_file, char* output_file, char* value_file, char* transient_file, char* parsed_value, char* evaluated_value), where:
    • input_file: absolute path of file being compiled.
    • output_file absolute path to user intended output file.
    • value_file absolute path to file which encountered the value.
    • transient_file in case where, for example, a mixin or a variable is defined, this path is the file containing the mixin and variable and it could be different that the value_file where it is ultimately consumed. Note import and implode may have different implications in path resolution. 😄
    • parsed_value raw value encountered by the parser
    • evaluated_value evaluated value, which is written to output today.
  • Return evaluated_value as is, if one of the following is encountered (defensive programming):
    • Absolute Path
    • URL
    • CSS4 variable.
    • Any invalid path character (which could be meant for post-processing)
  • Resolve and return relative path as: relative(input_file, relative(value_file, evaluated_value)).
• Expose same signature on API (perhaps as path_handler), so down-streams can override the default behavior.

[xzyfer]

I have a fix for this. Will publish when I'm home.