[Password Manager] Changes implementation of left elided origin on chrome://settings/passwords

[Password Manager] Changes implementation of left elided origin on chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of origin's string.

BUG=595662, 595276
Committed: https://crrev.com/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb
Cr-Commit-Position: refs/heads/master@{#382806}

[kolos1, 2016-03-21 13:08:15]

kolos@chromium.org changed reviewers:
+ estade@chromium.org

[kolos1, 2016-03-21 13:08:15]

Hi Evan, 

Please review this Javascript implementation of left elided origins for
chrome://settings/passwords. 

Regards,
Maxim

[kolos1, 2016-03-21 14:21:54]

Patchset #1 (id:1) has been deleted

[kolos1, 2016-03-21 14:22:32]

Description was changed from

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjust the length of
origin's string.

BUG=595662,595276
==========

to

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of
origin's string.

BUG=595662,595276
==========

[Evan Stade, 2016-03-21 21:59:02]

lgtm

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/password_manager.js (right):

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:179:
parseInt(window.getComputedStyle(entry.urlDiv).webkitPaddingStart);
nit: store getComputedStyle in a var?

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:186: urlLink.textContent =
'…' + urlLink.textContent.substring(2);
I kinda liked the idea of skipping ahead to the next . in the origin

[kolos1, 2016-03-22 12:40:12]

Patchset #2 (id:40001) has been deleted

[kolos1, 2016-03-22 19:10:24]

I made some changes. See comments. If this CL is ok, please review the next one
https://codereview.chromium.org/1615653005. It is upstreamed to this CL. Thanks.

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/password_manager.js (right):

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:179:
parseInt(window.getComputedStyle(entry.urlDiv).webkitPaddingStart);
On 2016/03/21 21:59:02, Evan Stade wrote:
> nit: store getComputedStyle in a var?

Done.

https://codereview.chromium.org/1817053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:186: urlLink.textContent =
'…' + urlLink.textContent.substring(2);
On 2016/03/21 21:59:02, Evan Stade wrote:
> I kinda liked the idea of skipping ahead to the next . in the origin

I asked UI team about this. They recommend to show as much as possible.

[Evan Stade, 2016-03-22 19:42:23]

https://codereview.chromium.org/1817053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/password_manager.js (right):

https://codereview.chromium.org/1817053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:185: urlLink.textContent =
'…' + urlLink.textContent.substring(1);
to maintain the tooltip, do you need urlLink.title = fullorigin?

[kolos1, 2016-03-22 19:49:26]

https://codereview.chromium.org/1817053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/password_manager.js (right):

https://codereview.chromium.org/1817053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/password_manager.js:185: urlLink.textContent =
'…' + urlLink.textContent.substring(1);
On 2016/03/22 19:42:23, Evan Stade wrote:
> to maintain the tooltip, do you need urlLink.title = fullorigin?

tootip is initialized with entry.url (fullorigin)

[Evan Stade, 2016-03-22 20:02:13]

lgtm

[kolos1, 2016-03-23 07:08:02]

The CQ bit was checked by kolos@chromium.org

[commit-bot: I haz the power, 2016-03-23 07:08:10]

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

[commit-bot: I haz the power, 2016-03-23 07:55:02]

Description was changed from

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of
origin's string.

BUG=595662,595276
==========

to

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of
origin's string.

BUG=595662,595276
==========

[commit-bot: I haz the power, 2016-03-23 07:55:03]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-03-23 07:56:09]

Description was changed from

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of
origin's string.

BUG=595662,595276
==========

to

==========
[Password Manager] Changes implementation of left elided origin on
chrome://settings/passwords

The previous implementation reversed the direction of text that causes bugs with
screen readers, copying the origin.

This CL introduces Javascript solution that dinamically adjusts the length of
origin's string.

BUG=595662,595276

Committed: https://crrev.com/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb
Cr-Commit-Position: refs/heads/master@{#382806}
==========

[commit-bot: I haz the power, 2016-03-23 07:56:10]

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

[kolos1, 2016-03-23 09:34:40]

A revert of this CL (patchset #2 id:60001) has been created in
https://codereview.chromium.org/1826533003/ by kolos@chromium.org.

The reason for reverting is: There is a bug in updateOriginsEliding_. The
algorithm might fall to infinite loop. 

CL also breaks "Closure Compilation Linux"
(https://build.chromium.org/p/chromium.fyi/builders/Closure%20Compilation%20Li...).
It complains on parseInt calls. parseInt should be called with 2 arguments
(parsed string and base).

.