FR: SAML Sign In - Interstitial page to send users directly to IdP login screen

chrome/browser/resources/chromeos/login/saml_interstitial.js

+// Copyright 2016 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.
+
+Polymer({
+  is: 'saml-interstitial',

[xiyuan, 2016/03/28 20:58:40]

nit: insert an empty line between all fields

[afakhry, 2016/03/28 23:15:32]

Done.

+  properties: {
+    domain: {
+      type: String,
+      observer: 'onDomainChanged_'
+    },
+    showDomainMessages_: {
+      type: Boolean,
+      value: false
+    },
+  },
+  i18n_: function(args) {
+    return loadTimeData.getStringF.apply(loadTimeData, args);
+  },
+  onDomainChanged_: function() {
+    this.$.message.content =
+      this.i18n_(['samlInterstitialMessage', this.domain]);

[xiyuan, 2016/03/28 20:58:40]

Why not just use loadTimeData here?

i.e.
  this.$.message.content = loadTimeData.getStringF('samlInterstitialMessage',
this.domain);

[afakhry, 2016/03/28 23:15:32]

Thanks this is certainly better. I saw an example that does this:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
so I thought this is the way.
Done.

[xiyuan, 2016/03/29 02:10:49]

I suspect the sample does it that way because of its content is provided as
polymer binding. But we do it in plain js so should be good to just use the old
way.

+    this.showDomainMessages_ = !!this.domain.length;

[xiyuan, 2016/03/28 20:58:40]

|showDomainMessages_| seems redundant. If we do want it, please document it in
html.

[afakhry, 2016/03/28 23:15:32]

I added a comment to the html.

+  },
+  onSamlPageNextClicked_: function() {
+    this.dispatchEvent(new Event('samlPageNextClicked'));

[xiyuan, 2016/03/28 20:58:40]

nit: this.fire('samlPageNextClicked');

[afakhry, 2016/03/28 23:15:32]

Done.

+  },
+  onSamlPageChangeAccountClicked_: function() {
+    this.dispatchEvent(new Event('samlPageChangeAccountClicked'));

[xiyuan, 2016/03/28 20:58:40]

nit: this.fire('samlPageChangeAccountClicked');

[afakhry, 2016/03/28 23:15:32]

Done.

+  },
+});