Submit button incompatiable with desktop chrome.

Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop chrome. They should at the very least fix the bugs where the submit button shows up. I have confirmed they work as before on the chromeos side.

BUG=643066, 643069, 644325, 644843
Committed: https://crrev.com/8b3ff5ba9c14e072c179fd3cb54636c499bc4dd4
Cr-Commit-Position: refs/heads/master@{#418384}

[sammiequon, 2016-09-08 22:15:39]

Description was changed from

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843
==========

to

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843
==========

[sammiequon, 2016-09-08 22:15:39]

sammiequon@chromium.org changed reviewers:
+ jdufault@chromium.org, xiyuan@chromium.org

[sammiequon, 2016-09-08 22:16:02]

jdufault@, xiyuan@ - Please take a look. Thanks!

[jdufault, 2016-09-08 22:28:59]

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_row.css (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_row.css:166: </if>
Is there an if/else construct available?

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_template.html (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_template.html:53: <if expr="chromeos">
Should this be cros only? It looks like we have had the icon on desktop for a
few years.

If this should be cros only, then the entire capslock-hint-container should
probably be disabled.

[xiyuan, 2016-09-08 22:36:29]

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_row.css (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_row.css:166: </if>
On 2016/09/08 22:28:59, jdufault wrote:
> Is there an if/else construct available?

I don't think grit support an "else". :( But please move the two <if>s together
so that it is easier to read.

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_template.html (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_template.html:53: <if expr="chromeos">
On 2016/09/08 22:28:59, jdufault wrote:
> Should this be cros only? It looks like we have had the icon on desktop for a
> few years.
> 
> If this should be cros only, then the entire capslock-hint-container should
> probably be disabled.

We probably have the load error since the code is ported to desktop chrome.
Don't think the caps lock feature is used on desktop chrome. So the whole
container can probably go.

[sammiequon, 2016-09-09 16:06:35]

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_row.css (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_row.css:166: </if>
On 2016/09/08 22:36:29, xiyuan wrote:
> On 2016/09/08 22:28:59, jdufault wrote:
> > Is there an if/else construct available?
> 
> I don't think grit support an "else". :( But please move the two <if>s
together
> so that it is easier to read.

Done.

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
File ui/login/account_picker/user_pod_template.html (right):

https://codereview.chromium.org/2326833002/diff/1/ui/login/account_picker/use...
ui/login/account_picker/user_pod_template.html:53: <if expr="chromeos">
On 2016/09/08 22:36:29, xiyuan wrote:
> On 2016/09/08 22:28:59, jdufault wrote:
> > Should this be cros only? It looks like we have had the icon on desktop for
a
> > few years.
> > 
> > If this should be cros only, then the entire capslock-hint-container should
> > probably be disabled.
> 
> We probably have the load error since the code is ported to desktop chrome.
> Don't think the caps lock feature is used on desktop chrome. So the whole
> container can probably go.

The caps lock feature is still inside the password box if caps lock is
activated, though there is already a caps lock indicator on the bottom right.

[xiyuan, 2016-09-09 18:05:45]

lgtm but wait for jdufault

[jdufault, 2016-09-13 18:26:50]

https://codereview.chromium.org/2326833002/diff/20001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.css (right):

https://codereview.chromium.org/2326833002/diff/20001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.css:260: <if expr="chromeos">
Can this entire css block be conditional since the capslock-hint-container
doesn't exist on non-cros?

[sammiequon, 2016-09-13 19:24:54]

https://codereview.chromium.org/2326833002/diff/20001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.css (right):

https://codereview.chromium.org/2326833002/diff/20001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.css:260: <if expr="chromeos">
On 2016/09/13 18:26:50, jdufault wrote:
> Can this entire css block be conditional since the capslock-hint-container
> doesn't exist on non-cros?

Done.

[jdufault, 2016-09-13 19:29:19]

lgtm

[sammiequon, 2016-09-13 20:31:56]

The CQ bit was checked by sammiequon@chromium.org

[sammiequon, 2016-09-13 20:31:56]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2326833002/#ps40001
(title: "Wrapped whole css block.")

[commit-bot: I haz the power, 2016-09-13 20:32:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 20:35:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 20:35:38]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sammiequon, 2016-09-13 21:04:45]

The CQ bit was checked by sammiequon@chromium.org

[sammiequon, 2016-09-13 21:04:46]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
jdufault@chromium.org
Link to the patchset: https://codereview.chromium.org/2326833002/#ps80001
(title: "Added null check.")

[commit-bot: I haz the power, 2016-09-13 21:05:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 21:55:37]

Description was changed from

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843
==========

to

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843
==========

[commit-bot: I haz the power, 2016-09-13 21:55:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-13 21:57:53]

Description was changed from

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843
==========

to

==========
Submit button incompatiable with desktop chrome.

Due to some complications I was not actually reproduce these bugs on desktop
chrome. They should at the very least fix the bugs where the submit button shows
up. I have confirmed they work as before on the chromeos side.

BUG=643066,643069,644325,644843

Committed: https://crrev.com/8b3ff5ba9c14e072c179fd3cb54636c499bc4dd4
Cr-Commit-Position: refs/heads/master@{#418384}
==========

[commit-bot: I haz the power, 2016-09-13 21:57:54]

Patchset 5 (id:??) landed as
https://crrev.com/8b3ff5ba9c14e072c179fd3cb54636c499bc4dd4
Cr-Commit-Position: refs/heads/master@{#418384}