This updates the StaticIP configuration UI to match new mocks.

chrome/browser/resources/options2/chromeos/internet_detail_ip_address_field.js

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+

[Dan Beam, 2012/08/11 01:38:02]

nit: one less \n here, IMO

+
+cr.define('options.internet', function() {
+  /** @const */ var EditableTextField = options.EditableTextField;
+
+  /**
+   * The regular expression that matches an IP address, allowing for
+   * leading/trailing/embedded whitespace.
+   * @const
+   * @private

[Dan Beam, 2012/08/11 01:38:02]

don't really need @private if this isn't a member of a class

+   * @type {RegExp}
+   */
+  var singleIp_ = new RegExp('^\\s*([0-9]+)\\s*\\.\\s*([0-9]+)\\s*\\.' +

[Dan Beam, 2012/08/11 01:38:02]

I'd highly recommend writing this in literal form and condensing:

  var singleIp_ = /^\s*(?:(\d{1,3})\s*\.\s*){3}(\d{1,3})\s*$/;

or stripping [^0-9.] characters and testing against a simpler RegExp:

  function prepIPForCheck(str) {
    return str.replace(/[^0-9.]/g, '');  // Implicitly trims.
  }
  var singleIp_ = /^(?:(\d{1,3})\.){3}(\d{1,3})$/;

(if that's possible)

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Yeah, I tried that already, but the problem is that then the match object only
returns the last of the first three octets instead of all three (yes, even with
the (?:), sadly).  And I went with the "new RegExp" form instead of the literal
form because I couldn't figure out how to split the literal form across lines.

I'll try splitting out the whitespace removal (I don't like having non-literal
RegExps either!).  Your way won't work though: I can't remove non-whitespace
characters when doing a validity check.  I'm just removing whitespace instead,
because I want that to be largely irrelevant.

[Dan Beam, 2012/08/13 19:48:55]

were you passing the /g (global) flag to the RegExp and/or making it non-greedy
with "?"?

"?" is the non-greedy modifier to "+" operator, means the match will have to
occur more than once but don't keep consuming greedily, .e.g. 'aaa'.match(/a+/g)
vs. 'aaa'.match(/a+?/g))?

[Greg Spencer (Chromium), 2012/08/13 22:17:44]

Yeah, I'm pretty familiar with regexps, and no, I wasn't using ? or /g. The code
you gave above returns the last one only:

-> var singleIp_ = /^(?:(\d{1,3})\.){3}(\d{1,3})$/;
-> singleIp_.exec("111.222.333.444")
["111.222.333.444", "333", "444"]

And variations on that don't change things:

-> var singleIp_ = /^(?:(\d{1,3})\.){3}?(\d{1,3})$/;
-> singleIp_.exec("111.222.333.444")
["111.222.333.444", "333", "444"]
-> var singleIp_ = /^(?:(\d{1,3})\.)+?(\d{1,3})$/;
-> singleIp_.exec("111.222.333.444")
["111.222.333.444", "333", "444"]
-> var singleIp_ = /^((\d{1,3})\.)+?(\d{1,3})$/;
-> singleIp_.exec("111.222.333.444")
["111.222.333.444", "333.", "333", "444"]
-> var singleIp_ = /^(?:(\d{1,3})\.)+(\d{1,3})$/g;
-> singleIp_.exec("111.222.333.444")
["111.222.333.444", "333", "444"]

The result I need is:
["111.222.333.444", "111", "222", "333", "444"]

The problem is that the regexp engine will only return one result in the match
array for each opening "(", regardless of how many times it matches.

[Dan Beam, 2012/08/13 22:56:18]

Yeah, OK.

+      '\\s*([0-9]+)\\s*\\.\\s*([0-9]+)\\s*$');
+
+  /**
+   * Creates a new field specifically for entering IP addresses.
+   * @constructor
+   */
+  function IPAddressField() {
+    var el = cr.doc.createElement('div');
+    IPAddressField.decorate(el);
+    return el;
+  };

[Dan Beam, 2012/08/11 01:38:02]

nit: no ; when using function declarations() { }

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+
+  /**
+   * Decorates an element as a inline-editable list item. Note that this is
+   * a subclass of IPAddressField.
+   * @param {!HTMLElement} el The element to decorate.
+   */
+  IPAddressField.decorate = function(el) {
+    el.__proto__ = IPAddressField.prototype;
+    el.decorate();
+  };
+
+  IPAddressField.prototype = {
+    __proto__: EditableTextField.prototype,
+
+    /** @inheritDoc */

[Dan Beam, 2012/08/11 01:38:02]

@inheritDoc is deprecated now, use @override instead

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+    decorate: function() {
+      EditableTextField.prototype.decorate.call(this);
+    },
+
+    /**
+     * Indicates whether or not empty values are allowed.
+     * @type {boolean}
+     */
+    get allowEmpty() {
+      return this.hasAttribute('allow-empty');
+    },
+
+    /** @inheritDoc */
+    get currentInputIsValid() {
+      if (!this.editField.value && this.allowEmpty)
+        return true;
+
+      // Make sure it's only got numbers and ".", there are the correct
+      // count of them, and they are all within the correct range.
+      var matches = singleIp_.exec(this.editField.value);
+      var range_correct = true;

[Dan Beam, 2012/08/11 01:38:02]

var rangeCorrent (camelCase in JavaScript)

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

aaagh!  Switching back and forth between C++ and JavaScript can drive one mad.
Done.

+      if (matches != null) {
+        for (var i = 1; i < matches.length; ++i) {
+          var value = parseInt(matches[i], 10);
+          if (value < 0 || value > 255)
+            range_correct = false;

[Dan Beam, 2012/08/11 01:38:02]

why not break; here?

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Why indeed.  Done.

+        }
+      }
+      return (this.editField.validity.valid && matches != null &&

[Dan Beam, 2012/08/11 01:38:02]

nit: less ()

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+              range_correct && matches.length == 5);
+    },
+
+    /** @inheritDoc */
+    get hasBeenEdited() {
+      return this.editField.value != this.model.value;
+    },
+
+    /**
+     * Mutates the input during a successful commit.  For the purposes of

[Dan Beam, 2012/08/11 01:38:02]

nit: 1 \s between sentences in comments (you have 2 \s here)

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+     * entering IP addresses, this just means stripping off whitespace and
+     * leading zeros from each of the octets so that they conform to the normal
+     * format for IP addresses.
+     * @param {Object} value Input IP address to be mutated.

[Dan Beam, 2012/08/11 01:38:02]

shouldn't @param and @return be a {string}?

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Yeah, I guess so.  I was originally going to mutate the model, but now just the
input string.  Done.

+     * @return {Object} Mutated IP address.
+     */
+    mutateInput: function(value) {
+      if (!value)
+        return value;

[Dan Beam, 2012/08/11 01:38:02]

nit: \n after this if ()

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+      var model = this.model;
+      var matches = singleIp_.exec(value);

[Dan Beam, 2012/08/11 01:38:02]

seems like you could simplify this to:
  
  return (value.match(/(\d+)/) || []).map(function() {
    return parseInt(a, 10);
  }).join('.');

ability to deal with hackiness may be required, though... (to like this, :P)

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Well, and the fact that it doesn't quite work.. :)

The regexp in the match would have to be my regexp, since the one you have only
returns the first octet (twice, since match() returns the whole match first
too).

In any case, I think I prefer the verbose version: it's easy to follow what's
happening, and it's not really any slower.  I'll simplify it as you suggest
below.

+      var result = [];
+      if (matches != null) {  // should never be null if we got this far

[Dan Beam, 2012/08/11 01:38:02]

make a full sentence if this loop sticks around

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+        for (var i = 1; i < matches.length; ++i) {
+          var octet = parseInt(matches[i], 10);
+          result.push(octet);

[Dan Beam, 2012/08/11 01:38:02]

inline octect if this loop sticks around

[Greg Spencer (Chromium), 2012/08/13 19:20:04]

Done.

+        }
+      }
+      return result.join('.');
+    },
+  };
+
+  return {
+    IPAddressField: IPAddressField,
+  };
+});