Hi there,

I'm trying to get involved with chromium development. I am actually familiar with this source code. May I please take on this ticket?

Cheers

Hi, I already started working on a patch myself, but feel free to have a go at it. I'm happy to review it once you have a first version ready.

I'll stay the owner, as it seems impossible to assign a bug to non @chromium.org email addresses.

Ambiguity is being cause by multiple entries based for the same user in the same url, however using element with different names from the original entry. I sorted it out by forwarding this information to the password list, in a way that we can refer back to it when showing a password, or deleting an saved password. It works fine, but I'm not sure if that was the design you were looking for, so I can change it if you have a better approach in mind.

I'll be pushing the code first thing in the morning.

I also came across a bug that is unrelated to this specific duplication issue. Basically, if you delete a line that has the password in plain view, the correct line is removed from the password database, however the UI merges whatever line is being shown below to the line that has just been deleted in way that the deleted password stays in plain view but now as if it was being used for the following line. I have tried to fix it, to no avail... because I'm C++ developer and I believe the problem is on the following html lines https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/passwords_and_forms_page/password_list_item.html?l=63&rcl=95b827c4f05395c048567567fad603fecde01388

As you can see, the federationtext is in a separate section, and I believe it is left behind when the PasswordManager removes that line from the GUI.

I can fix it if you want me to, but I would need some directions.

Finally, I would appreciate any directions on what kind of tests I should write for these changes.

Cheers

https://chromium-review.googlesource.com/c/chromium/src/+/666617

Check it out!

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

commit 1bbb55a9c4e6765685210a3b4ffe94f5104ef031
Author: Claudio DeSouza <claudiomdsjr@gmail.com>
Date: Thu Sep 21 12:11:40 2017

Password entries in settings being indexed to avoid ambiguity

Password entries shown in settings are stored with many other pieces of
informations besides username and url. However, since most of the
lookups coming from settings would take into account only these two
pieces of data. We are now using an index based on the order that the
list is generated to refer to elements when querying their password
text or just removing them.

R=hcarmona@chromium.org, jdoerrie@chromium.org, stevenjb@chromium.org, tbarzic@chromium.org

Bug:  764667 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib5bafd10a7bed25e27c0ffc00b24f7a9e1f77d48
Reviewed-on: https://chromium-review.googlesource.com/666617
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503420}
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/AUTHORS
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/android/password_ui_view_android.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/android/password_ui_view_android.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/extensions/api/passwords_private/passwords_private_utils.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/browser/ui/passwords/password_ui_view.h
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/common/extensions/api/passwords_private.idl
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/test/data/extensions/api_test/passwords_private/test.js
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/test/data/webui/settings/passwords_and_autofill_fake_data.js
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
[modify] https://crrev.com/1bbb55a9c4e6765685210a3b4ffe94f5104ef031/third_party/closure_compiler/externs/passwords_private.js