Bug 49848

Summary: Make it possible to display the last character of a secure text field unobscured
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Web Template FrameworkAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch darin: review+

Description David Kilzer (:ddkilzer) 2010-11-19 18:03:22 PST
Created attachment 74451 [details]
Patch

Reviewed by NOBODY (OOPS!).

* JavaScriptCore.exp:
* wtf/text/StringImpl.cpp:
(WTF::StringImpl::secure): Added argument that controls whether
the last character is obscured or not.  Implemented behavior.
* wtf/text/StringImpl.h:
(WTF::StringImpl::LastCharacterBehavior): Added enum.
(WTF::StringImpl::secure): Updated method signature.
---
 4 files changed, 26 insertions(+), 5 deletions(-)
Comment 1 Darin Adler 2010-11-19 20:35:59 PST
Comment on attachment 74451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74451&action=review

> JavaScriptCore/wtf/text/StringImpl.cpp:277
> +    if (m_length) {

I think it would be better to say:

    if (!m_length)
        return this;

At the start of the function, even before the createUninitialized call.

> JavaScriptCore/wtf/text/StringImpl.cpp:278
> +        const unsigned lastCharacterIndex = m_length - 1;

We normally don’t use const for local variables like this one.
Comment 2 Darin Adler 2010-11-19 20:36:20 PST
Comment on attachment 74451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74451&action=review

> JavaScriptCore/wtf/text/StringImpl.h:290
> +    PassRefPtr<StringImpl> secure(UChar, LastCharacterBehavior behavior = HideLastCharacter);

We could omit the argument name “behavior” here.
Comment 3 David Kilzer (:ddkilzer) 2010-11-20 11:03:14 PST
Committed r72477: <http://trac.webkit.org/changeset/72477>