New issue
Advanced search Search tips

Issue 595662 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Improve implementation of left elided origins on chrome://settings/passwords

Project Member Reported by kolos@chromium.org, Mar 17 2016

Issue description

At this point, left elided origins are implemented by reversing direction of text (https://codereview.chromium.org/1318523011/). Unnatural direction of the text causes other tricky changes.

 

Comment 1 by kolos@chromium.org, Mar 17 2016

Cc: kolos@chromium.org
One of alternative solutions is javascript text eliding. I tried some javascript solutions, but I believe it is much more tricky then adding CSS classes. 

Comment 2 by kolos@chromium.org, Mar 21 2016

Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb

commit 1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb
Author: kolos <kolos@chromium.org>
Date: Wed Mar 23 07:54:51 2016

[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 

Review URL: https://codereview.chromium.org/1817053002

Cr-Commit-Position: refs/heads/master@{#382806}

[modify] https://crrev.com/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb/chrome/browser/resources/options/password_manager.js
[modify] https://crrev.com/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb/chrome/browser/resources/options/password_manager_list.css
[modify] https://crrev.com/1f3fdc9a5caf59ff07ac0edc37123d17ee0640bb/chrome/browser/resources/options/password_manager_list.js

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49fe82d1638da17aa71be0e6a7fb5f7c18deba70

commit 49fe82d1638da17aa71be0e6a7fb5f7c18deba70
Author: kolos <kolos@chromium.org>
Date: Wed Mar 23 09:35:09 2016

Revert of [Password Manager] Changes implementation of left elided origin on chrome://settings/passwords (patchset #2 id:60001 of https://codereview.chromium.org/1817053002/ )

Reason for revert:
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%20Linux/builds/50842). It complains on parseInt calls. parseInt should be called with 2 arguments (parsed string and base).

Original issue's description:
> [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}

TBR=estade@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 595662 , 595276 

Review URL: https://codereview.chromium.org/1826533003

Cr-Commit-Position: refs/heads/master@{#382814}

[modify] https://crrev.com/49fe82d1638da17aa71be0e6a7fb5f7c18deba70/chrome/browser/resources/options/password_manager.js
[modify] https://crrev.com/49fe82d1638da17aa71be0e6a7fb5f7c18deba70/chrome/browser/resources/options/password_manager_list.css
[modify] https://crrev.com/49fe82d1638da17aa71be0e6a7fb5f7c18deba70/chrome/browser/resources/options/password_manager_list.js

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab

commit fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab
Author: kolos <kolos@chromium.org>
Date: Wed Mar 30 07:52:40 2016

[Password Manager] Changes implementation of left elided origins on chrome://settings/passwords (Relanding)

Reland https://codereview.chromium.org/1826533003/. It broke Closure Linux Compilation bot (https://build.chromium.org/p/chromium.fyi/builders/Closure%20Compilation%20Linux/builds/50842). The bot complained on parseInt calls. parseInt should be called with 2 arguments (parsed string and base).

There was also a bug in the algorithm of updateOriginsEliding_. Reproduction: user enters a query w/o any matching entries in search box (i.e. there will be no entries) and then removes the query. The algorithm falls into infinite loop, because entry.urlDiv.offsetWidth is 0. We have to swap updateOriginsEliding_ and updateListVisibility_ to update the list before we read entry.urlDiv.offsetWidth.

BUG= 595662 ,  595276 

Review URL: https://codereview.chromium.org/1823423002

Cr-Commit-Position: refs/heads/master@{#383927}

[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager.js
[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager_list.css
[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager_list.js

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab

commit fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab
Author: kolos <kolos@chromium.org>
Date: Wed Mar 30 07:52:40 2016

[Password Manager] Changes implementation of left elided origins on chrome://settings/passwords (Relanding)

Reland https://codereview.chromium.org/1826533003/. It broke Closure Linux Compilation bot (https://build.chromium.org/p/chromium.fyi/builders/Closure%20Compilation%20Linux/builds/50842). The bot complained on parseInt calls. parseInt should be called with 2 arguments (parsed string and base).

There was also a bug in the algorithm of updateOriginsEliding_. Reproduction: user enters a query w/o any matching entries in search box (i.e. there will be no entries) and then removes the query. The algorithm falls into infinite loop, because entry.urlDiv.offsetWidth is 0. We have to swap updateOriginsEliding_ and updateListVisibility_ to update the list before we read entry.urlDiv.offsetWidth.

BUG= 595662 ,  595276 

Review URL: https://codereview.chromium.org/1823423002

Cr-Commit-Position: refs/heads/master@{#383927}

[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager.js
[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager_list.css
[modify] https://crrev.com/fa2ae1d61d3bb8d61e6bf2aee4d17662925d70ab/chrome/browser/resources/options/password_manager_list.js

Comment 7 by kolos@chromium.org, Mar 30 2016

Status: Fixed (was: Started)

Sign in to add a comment