Make the URLs in the password exceptions list clickable.

Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
Committed: https://crrev.com/f01343295501f5113c8b5a97d9008a9a95aa894a
Cr-Commit-Position: refs/heads/master@{#389533}

[hcarmona, 2016-04-22 00:30:55]

Patchset #2 (id:10001) has been deleted

[hcarmona, 2016-04-22 00:31:08]

Patchset #1 (id:1) has been deleted

[hcarmona, 2016-04-22 01:38:48]

Description was changed from

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
==========

to

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
==========

[hcarmona, 2016-04-22 01:38:48]

hcarmona@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, stevenjb@chromium.org

[hcarmona, 2016-04-22 01:46:23]

PTAL, extending the API to get click-able URLs.

[Devlin, 2016-04-22 15:21:09]

Do you need me to look at anything other than the idl?  If not, lg so far, but
waiting to see if Steven has any comments that will change it.

[stevenjb, 2016-04-22 16:11:20]

https://codereview.chromium.org/1907813002/diff/30001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
(right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc:52:
exception.link_url = ss.str();
nit: we should probably test with a different link url

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
File chrome/common/extensions/api/passwords_private.idl (right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
chrome/common/extensions/api/passwords_private.idl:22: // The URL that should be
linked to when clicking in the UI.
We should update this comment also (see comment below).

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
chrome/common/extensions/api/passwords_private.idl:37: // The URL that should be
linked to when clicking in the UI.
nit: Something like "The complete URL of the page where the exception applies."
We shouldn't describe a particular implementation in the API.

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/passwords_private/test.js (right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/passwords_private/test.js:82:
chrome.test.assertTrue(!!list);
Ideally we should populate the list in the test/fake implementation (or use well
known values like in line 67) and test the value of each url for at least one
entry.

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
(right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_passwords_section_browsertest.js:62:
linkUrl: url,
Test with different values, e.g. append "/login"

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_passwords_section_browsertest.js:102:
node.querySelector('#exception').textContent);
Test the link url also

[hcarmona, 2016-04-23 01:46:08]

https://codereview.chromium.org/1907813002/diff/30001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
(right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc:52:
exception.link_url = ss.str();
On 2016/04/22 16:11:20, stevenjb wrote:
> nit: we should probably test with a different link url

Done.

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
File chrome/common/extensions/api/passwords_private.idl (right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
chrome/common/extensions/api/passwords_private.idl:22: // The URL that should be
linked to when clicking in the UI.
On 2016/04/22 16:11:20, stevenjb wrote:
> We should update this comment also (see comment below).

Done.

https://codereview.chromium.org/1907813002/diff/30001/chrome/common/extension...
chrome/common/extensions/api/passwords_private.idl:37: // The URL that should be
linked to when clicking in the UI.
On 2016/04/22 16:11:20, stevenjb wrote:
> nit: Something like "The complete URL of the page where the exception
applies."
> We shouldn't describe a particular implementation in the API.
> 

Sounds good, updated comments.

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/passwords_private/test.js (right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/passwords_private/test.js:82:
chrome.test.assertTrue(!!list);
On 2016/04/22 16:11:20, stevenjb wrote:
> Ideally we should populate the list in the test/fake implementation (or use
well
> known values like in line 67) and test the value of each url for at least one
> entry.
> 

Done-ish. I've validated that the list has more than 0 items and the
URLs are also not empty.

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
(right):

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_passwords_section_browsertest.js:62:
linkUrl: url,
On 2016/04/22 16:11:20, stevenjb wrote:
> Test with different values, e.g. append "/login"

Done.

https://codereview.chromium.org/1907813002/diff/30001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_passwords_section_browsertest.js:102:
node.querySelector('#exception').textContent);
On 2016/04/22 16:11:20, stevenjb wrote:
> Test the link url also

Done.

[stevenjb, 2016-04-25 16:22:18]

lgtm

[Devlin, 2016-04-25 16:25:30]

idl lgtm

[hcarmona, 2016-04-25 18:17:32]

The CQ bit was checked by hcarmona@chromium.org

[commit-bot: I haz the power, 2016-04-25 18:17:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907813002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907813002/50001

[commit-bot: I haz the power, 2016-04-25 19:22:57]

Description was changed from

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
==========

to

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
==========

[commit-bot: I haz the power, 2016-04-25 19:22:58]

Committed patchset #2 (id:50001)

[commit-bot: I haz the power, 2016-04-25 19:24:36]

Description was changed from

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538
==========

to

==========
Make the URLs in the password exceptions list clickable.

Required updating the passwords private API to return the link URL.

No UI change. Links behave the same as the saved password URL links by
underlining only on hover.

BUG=595538

Committed: https://crrev.com/f01343295501f5113c8b5a97d9008a9a95aa894a
Cr-Commit-Position: refs/heads/master@{#389533}
==========

[commit-bot: I haz the power, 2016-04-25 19:24:38]

Patchset 2 (id:??) landed as
https://crrev.com/f01343295501f5113c8b5a97d9008a9a95aa894a
Cr-Commit-Position: refs/heads/master@{#389533}