Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(514)

Issue 6369015: DOMUI: Two fixes for CC editor. (Closed)

Created:
9 years, 11 months ago by James Hawkins
Modified:
9 years, 7 months ago
Reviewers:
Ilya Sherman, dhollowa
CC:
chromium-reviews, arv (Not doing code reviews)
Visibility:
Public.

Description

DOMUI: Two fixes for CC editor. * Only show the obfuscated CC number on editor load. * Clear the input field if the user modifies the obfuscated number. BUG=70452 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72577

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -12 lines) Patch
M chrome/browser/dom_ui/options/autofill_options_handler.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/autofill_edit_creditcard_overlay.js View 6 chunks +42 lines, -12 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
James Hawkins
9 years, 11 months ago (2011-01-26 00:47:23 UTC) #1
dhollowa
LGTM. Rubber stamp on JS style, but the logic looks good.
9 years, 11 months ago (2011-01-26 00:51:38 UTC) #2
Ilya Sherman
What if I edit the number using only the mouse (select all, paste)?
9 years, 11 months ago (2011-01-26 00:57:16 UTC) #3
Ilya Sherman
Also a behavioral bug: when adding a new credit card number, the first digit typed ...
9 years, 11 months ago (2011-01-26 00:59:08 UTC) #4
Ilya Sherman
9 years, 11 months ago (2011-01-26 01:07:58 UTC) #5
More verbose:

Why do we want the obfuscated number to magically disappear when the user begins
editing the number?  This seems unnecessarily magical and confusing -- if I want
to change the number, I know how to select the text and delete it before doing
so.

Independently of that, I find the "hasEditedNumber_" logic odd.  Two
alternatives that make more sense to me:
(1) Store the obfuscated number as well as the non-obfuscated number.  Determine
whether the field has been edited by comparing to the stored obfuscated value.
(2) Consider the field to have been editing only if all of the characters are
digits.

If you'd like to keep the current logic, though, I guess oninput() or onchange()
would be more appropriate than onkeydown().

Powered by Google App Engine
This is Rietveld 408576698