fix(input[range]): correctly handle min/max; remove ngMin/ngMax support

This commit fixes the handling of min/max, and removes support for ngMin/ngMax:

min/max:
Previously, interpolated min/max values on input range were not set when the first $render happens,
because the interpolation directive only sets the actual element attribute value after
a digest passes. That means that the browser would not adjust
the input value according to min/max and the range input and model would
not be initialzed as expected.
With this change, input range will set the actual element attribute value during its own
linking phase, as it is already available on the attrs argument passed to the link fn.

ngMin/ngMax
Since ng prefixed attributes do not set their corresponding element attribute, the range input would always have min = 0, and max = 100 (in supported browsers), regardless of the value
in ngMin/ngMax. This is confusing and not very useful, so it's better to not support these attributes at all.

The commit also fixes a test which used an interpolation inside an attribute that expects an expression.

Fixes angular#14982
PR (angular#14996)
 (reverted from commit b78539b)

Author: petebacondarwin

src/ng/directive/input.js

@@ -1061,18 +1061,23 @@ var inputType = {
    * Angular will also update the model value.
    *
    * Automatic value adjustment also means that a range input element can never have the `required`,
-   * `min`, or `max` errors.
-   *
-   * Note that `input[range]` is not compatible with`ngMax` and `ngMin`, because they do not set the
-   * `min` and `max` attributes, which means that the browser won't automatically adjust the input
-   * value based on their values, and will always assume min = 0 and max = 100.
+   * `min`, or `max` errors, except when using `ngMax` and `ngMin`, which are not affected by automatic
+   * value adjustment, because they do not set the `min` and `max` attributes.
    *
    * @param {string}  ngModel Assignable angular expression to data-bind to.
    * @param {string=} name Property name of the form under which the control is published.
    * @param {string=} min Sets the `min` validation to ensure that the value entered is greater
    *                  than `min`. Can be interpolated.
    * @param {string=} max Sets the `max` validation to ensure that the value entered is less than `max`.
    *                  Can be interpolated.
+   * @param {string=} ngMin Takes an expression. Sets the `min` validation to ensure that the value
+   *                  entered is greater than `min`. Does not set the `min` attribute and therefore
+   *                  adds no native HTML5 validation. It also means the browser won't adjust the
+   *                  element value in case `min` is greater than the current value.
+   * @param {string=} ngMax Takes an expression. Sets the `max` validation to ensure that the value
+   *                  entered is less than `max`. Does not set the `max` attribute and therefore
+   *                  adds no native HTML5 validation. It also means the browser won't adjust the
+   *                  element value in case `max` is less than the current value.
    * @param {string=} ngChange Angular expression to be executed when the ngModel value changes due
    *                  to user interaction with the input element.
    *
@@ -1542,12 +1547,10 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
   numberFormatterParser(ctrl);
   baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
 
-  var supportsRange = ctrl.$$hasNativeValidators && element[0].type === 'range',
-      minVal = supportsRange ? 0 : undefined,
-      maxVal = supportsRange ? 100 : undefined,
-      validity = element[0].validity,
-      hasMinAttr = isDefined(attr.min),
-      hasMaxAttr = isDefined(attr.max);
+  var minVal = 0,
+      maxVal = 100,
+      supportsRange = ctrl.$$hasNativeValidators && element[0].type === 'range',
+      validity = element[0].validity;
 
   var originalRender = ctrl.$render;
 
@@ -1560,39 +1563,6 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
     } :
     originalRender;
 
-  if (hasMinAttr) {
-    ctrl.$validators.min = supportsRange ?
-      // Since all browsers set the input to a valid value, we don't need to check validity
-      function noopMinValidator() { return true; } :
-      // non-support browsers validate the range
-      function minValidator(modelValue, viewValue) {
-        return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
-      };
-
-    setInitialValueAndObserver('min', minChange);
-  }
-
-  if (hasMaxAttr) {
-    ctrl.$validators.max = supportsRange ?
-      // Since all browsers set the input to a valid value, we don't need to check validity
-      function noopMaxValidator() { return true; } :
-      // ngMax doesn't set the max attr, so the browser doesn't adjust the input value as setting max would
-      function maxValidator(modelValue, viewValue) {
-        return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
-      };
-
-    setInitialValueAndObserver('max', maxChange);
-  }
-
-  function setInitialValueAndObserver(htmlAttrName, changeFn) {
-    // interpolated attributes set the attribute value only after a digest, but we need the
-    // attribute value when the input is first rendered, so that the browser can adjust the
-    // input value based on the min/max value
-    element.attr(htmlAttrName, attr[htmlAttrName]);
-
-    attr.$observe(htmlAttrName, changeFn);
-  }
-
   function minChange(val) {
     if (isDefined(val) && !isNumber(val)) {
       val = parseFloat(val);
@@ -1603,12 +1573,12 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
       return;
     }
 
-    if (supportsRange) {
+    if (supportsRange && minAttrType === 'min') {
       var elVal = element.val();
       // IE11 doesn't set the el val correctly if the minVal is greater than the element value
       if (minVal > elVal) {
+        element.val(minVal);
         elVal = minVal;
-        element.val(elVal);
       }
       ctrl.$setViewValue(elVal);
     } else {
@@ -1617,6 +1587,23 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
     }
   }
 
+  var minAttrType = isDefined(attr.ngMin) ? 'ngMin' : isDefined(attr.min) ? 'min' : false;
+  if (minAttrType) {
+    ctrl.$validators.min = isDefined(attr.min) && supportsRange ?
+      function noopMinValidator(value) {
+        // Since all browsers set the input to a valid value, we don't need to check validity
+        return true;
+      } :
+      // ngMin doesn't set the min attr, so the browser doesn't adjust the input value as setting min would
+      function minValidator(modelValue, viewValue) {
+        return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
+      };
+
+    // Assign minVal when the directive is linked. This won't run the validators as the model isn't ready yet
+    minChange(attr.min);
+    attr.$observe('min', minChange);
+  }
+
   function maxChange(val) {
     if (isDefined(val) && !isNumber(val)) {
       val = parseFloat(val);
@@ -1627,20 +1614,35 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
       return;
     }
 
-    if (supportsRange) {
+    if (supportsRange && maxAttrType === 'max') {
       var elVal = element.val();
       // IE11 doesn't set the el val correctly if the maxVal is less than the element value
       if (maxVal < elVal) {
         element.val(maxVal);
-        // IE11 and Chrome don't set the value to the minVal when max < min
-        elVal = maxVal < minVal ? minVal : maxVal;
+        elVal = minVal;
       }
       ctrl.$setViewValue(elVal);
     } else {
       // TODO(matsko): implement validateLater to reduce number of validations
       ctrl.$validate();
     }
   }
+  var maxAttrType = isDefined(attr.max) ? 'max' : attr.ngMax ? 'ngMax' : false;
+  if (maxAttrType) {
+    ctrl.$validators.max = isDefined(attr.max) && supportsRange ?
+      function noopMaxValidator() {
+        // Since all browsers set the input to a valid value, we don't need to check validity
+        return true;
+      } :
+      // ngMax doesn't set the max attr, so the browser doesn't adjust the input value as setting max would
+      function maxValidator(modelValue, viewValue) {
+        return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
+      };
+
+    // Assign maxVal when the directive is linked. This won't run the validators as the model isn't ready yet
+    maxChange(attr.max);
+    attr.$observe('max', maxChange);
+  }
 
 }