778389 - Password mask should be U+2022 on Android

Status: RESOLVED FIXED

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

Description

[Makoto Kato [:m_kato]]

Android password control uses U+2022 as password mask.  So we should use this character.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 2

[Chris Peterson [:cpeterson]]

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

Comment 3

[Makoto Kato [:m_kato]]

(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.

Comment 4

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e677fd2a66dd

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e677fd2a66dd