Closed Bug 778389 Opened 7 years ago Closed 7 years ago

Password mask should be U+2022 on Android

Categories

(Firefox for Android :: Keyboards and IME, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 17

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(1 file)

Android password control uses U+2022 as password mask.  So we should use this character.
Attached patch fixSplinter Review
Attachment #646806 - Flags: review?(blassey.bugs)
Comment on attachment 646806 [details] [diff] [review]
fix

Review of attachment 646806 [details] [diff] [review]:
-----------------------------------------------------------------

Makoto, blassey is out on vacation all next week, so I will volunteer to review your patch. :) Looks good to me, with a couple suggestions.

Does this change depend on new Gecko changes? Or can we uplift this patch to Aurora 16 (or even Beta 15)? Your fix is small and would improve Firefox's native look on Android.

::: widget/android/nsLookAndFeel.cpp
@@ +475,5 @@
>  
>  PRUint32
>  nsLookAndFeel::GetPasswordMaskDelayImpl()
>  {
>    // This value is hard-coded in PasswordTransformationMethod.java

Please add a comment like "This value is hard-coded in Android OS's PasswordTransformationMethod.java". When I first read this comment, I was not sure if this comment referred to a Mozilla file or a Google/Android file. <:)

@@ +483,5 @@
> +/* virtual */
> +PRUnichar
> +nsLookAndFeel::GetPasswordCharacterImpl()
> +{
> +  // This value is from PasswordTransformationMethod.java

Please expand this comment like above.

@@ +484,5 @@
> +PRUnichar
> +nsLookAndFeel::GetPasswordCharacterImpl()
> +{
> +  // This value is from PasswordTransformationMethod.java
> +  return 0x2022;

I suggest adding named const PRUnichar called UNICODE_BULLET, so readers know what a 0x2022 looks like. Defining a named const local to this function would be good enough.

"Bullet" is the official character name, according to this source:
http://www.fileformat.info/info/unicode/char/2022/index.htm
Attachment #646806 - Flags: review?(blassey.bugs) → review+
(In reply to Chris Peterson (:cpeterson) from comment #2)
> Comment on attachment 646806 [details] [diff] [review]
> fix
> 
> Review of attachment 646806 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Makoto, blassey is out on vacation all next week, so I will volunteer to
> review your patch. :) Looks good to me, with a couple suggestions.
> 
> Does this change depend on new Gecko changes? Or can we uplift this patch to
> Aurora 16 (or even Beta 15)? Your fix is small and would improve Firefox's
> native look on Android.

Yes.  But we need rebase for Aurora due to bug 772327.  After landing this, I will request this.
https://hg.mozilla.org/mozilla-central/rev/e677fd2a66dd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.