Last Comment Bug 655864 - Aurora: password field is not masked immediately and shows artifacts, when typing past the end of the field
: Aurora: password field is not masked immediately and shows artifacts, when ty...
Status: VERIFIED FIXED
:
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Firefox 5
: ARM Android
: P4 critical (vote)
: Firefox 5
Assigned To: Benjamin Stover (:stechz)
:
:
Mentors:
http://reader.google.com
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-09 15:57 PDT by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2013-12-27 14:23 PST (History)
9 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot (34.70 KB, image/png)
2011-05-11 15:59 PDT, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field (3.50 KB, patch)
2011-05-13 16:05 PDT, Benjamin Stover (:stechz)
roc: review+
Details | Diff | Splinter Review
(trunk patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field (2.75 KB, patch)
2011-05-13 16:27 PDT, Benjamin Stover (:stechz)
roc: review+
Details | Diff | Splinter Review
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field (1.65 KB, patch)
2011-05-17 13:29 PDT, Benjamin Stover (:stechz)
no flags Details | Diff | Splinter Review
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field (3.53 KB, patch)
2011-05-17 15:47 PDT, Benjamin Stover (:stechz)
no flags Details | Diff | Splinter Review
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field (3.53 KB, patch)
2011-05-17 15:48 PDT, Benjamin Stover (:stechz)
no flags Details | Diff | Splinter Review
Password field is not masked immediately and shows artifacts, when typing past the end of the field (7.98 KB, patch)
2011-05-17 16:33 PDT, Benjamin Stover (:stechz)
roc: review-
Details | Diff | Splinter Review
Password field is not masked immediately and shows artifacts, when typing past the end of the field (8.86 KB, patch)
2011-05-18 08:34 PDT, Benjamin Stover (:stechz)
roc: review+
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Straight lines between characters (31.21 KB, image/png)
2011-05-19 09:00 PDT, Anna (Waverley)
no flags Details

Description User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-05-09 15:57:15 PDT
Aurora Branch:
Mozilla/5.0 (Android; Linux armv71; rv5.0a2) Gecko/20110509 Firefox/5.0a2 Fennec/5.0a2
Device: Thunderbolt
OS: Android 2.2

1. go to http://reader.google.com
2. select the password field and type

Expected: the typing should get masked with a *
Actual: you can see the original letters being typed

Note:
1. the login doesn't look so great, esp if you type pass the boundary which also happens on the nightly.
Comment 1 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-05-11 15:59:29 PDT
Created attachment 531784 [details]
screenshot

I'm not sure why but the downloads from 2011-05-10-05-mozilla-aurora-android-r7/ doesn't show the issue, but the downloads from today latest-mozilla-aurora-android-r7/ does show the issue.  I had downloaded from the alias on 05/09.

Attached is a screenshot of what I see.  Is it a build issue?
Comment 2 User image Benjamin Stover (:stechz) 2011-05-13 14:58:24 PDT
That looks like the password field isn't getting invalidated for some reason.
Comment 3 User image Benjamin Stover (:stechz) 2011-05-13 15:00:49 PDT
This is not reproducing for me from a freshly checked out copy of aurora. Does this happen on Desktop?
Comment 4 User image Aaron Train [:aaronmt] 2011-05-13 15:05:48 PDT
For what it's worth, this is marked as Aurora but I can reproduce in 4.0.1
Comment 5 User image Matt Brubeck (:mbrubeck) 2011-05-13 15:15:31 PDT
I could not reproduce this in 4.0.1, Aurora, or Nightly using an HTC T-Mobile G2 with Android 2.2, using either the stock Android keyboard or Swype.
Comment 6 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-05-13 15:24:12 PDT
Need to keep typing past the field length in order to repro the issue.
Comment 7 User image Aaron Train [:aaronmt] 2011-05-13 15:25:07 PDT
Sorry, to clarify what I'm seeing looks like this http://i.imgur.com/HwKyG.png -- mine will mask after a delay .. Naoki, does yours eventually mask?
Comment 8 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-05-13 15:45:02 PDT
Yes, Aaron, mine eventually masks after a 2 to 3 second delay.
Comment 9 User image Matt Brubeck (:mbrubeck) 2011-05-13 15:51:01 PDT
Okay, I can reproduce this when typing past the end of the field in Aurora.

Given that this occurs in more limited circumstances than we first thought, and that the password does mask after a few seconds, this probably doesn't need to be a "hardblocker" for Fennec 5.
Comment 10 User image Benjamin Stover (:stechz) 2011-05-13 16:05:50 PDT
Created attachment 532374 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field
Comment 11 User image Benjamin Stover (:stechz) 2011-05-13 16:07:16 PDT
Roc, moving the info layer to below the overflow clip fixes this problem. I'm not really sure why. My best theory is that the info layer got in the way of clip overflow flattening, causing the invalidation to not happen properly.
Comment 12 User image Benjamin Stover (:stechz) 2011-05-13 16:27:09 PDT
Created attachment 532380 [details] [diff] [review]
(trunk patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field
Comment 13 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-14 02:28:17 PDT
Comment on attachment 532374 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field

Review of attachment 532374 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 14 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-14 02:28:34 PDT
Comment on attachment 532380 [details] [diff] [review]
(trunk patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field

Review of attachment 532380 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 15 User image Benjamin Stover (:stechz) 2011-05-16 10:41:17 PDT
Back to the drawing board. This patch effectively disables asynchronous scrolling for scroll frames altogether, so that's why it fixed the bug...
Comment 16 User image Benjamin Stover (:stechz) 2011-05-17 13:29:22 PDT
Created attachment 533048 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field
Comment 17 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-17 15:14:21 PDT
Comment on attachment 533048 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field

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

I think FrameLayerBuilder::DisplayItemDataEntry::HasContainerLayer also has to be modified to ignore your scroll info layers, preferably by capturing the ACTIVE_EMPTY state and storing it in the DisplayItemData.
Comment 18 User image Benjamin Stover (:stechz) 2011-05-17 15:47:53 PDT
Created attachment 533096 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field
Comment 19 User image Benjamin Stover (:stechz) 2011-05-17 15:48:17 PDT
Created attachment 533097 [details] [diff] [review]
(aurora patch) Password field is not masked immediately and shows artifacts, when typing past the end of the field

Not sure if this is more satisfactory, but I tried something different and
checked if the bit flag is set. This looks similar to what FrameLayerBuilder
does in StoreNewDisplayItemData so I thought it might work. Let me know if I
need to do this as per your suggestion.
Comment 20 User image Benjamin Stover (:stechz) 2011-05-17 16:33:21 PDT
Created attachment 533111 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

After talking on IRC, I now understand the above strategy wouldn't work because
the real issue is that the container layer flag will never get unset.
Comment 21 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-17 17:20:54 PDT
Comment on attachment 533111 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

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

r+ with those fixed

::: layout/base/FrameLayerBuilder.cpp
@@ +443,4 @@
>  {
>    for (PRUint32 i = 0; i < mData.Length(); ++i) {
> +    if (mData[i].mLayer->GetType() == Layer::TYPE_CONTAINER &&
> +        mData[i].mIsEmptyLayer)

!mData[i].mIsEmptyLayer, surely?

@@ +1382,5 @@
>        InvalidateForLayerChange(item, ownLayer);
>  
>        mNewChildLayers.AppendElement(ownLayer);
> +      mBuilder->LayerBuilder()->AddLayerDisplayItem(
> +        ownLayer, item, layerState == LAYER_ACTIVE_EMPTY);

Better to just pass in layerState directly. Boolean parameters suck!
Comment 22 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-17 17:21:39 PDT
In particular, instead of mIsEmptyLayer, just store the layer state and check it in HasNonEmptyContainerLayer.
Comment 23 User image Benjamin Stover (:stechz) 2011-05-18 08:34:34 PDT
Created attachment 533296 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field
Comment 24 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-18 14:40:41 PDT
Comment on attachment 533296 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

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

::: layout/base/FrameLayerBuilder.h
@@ +225,5 @@
>     * Record aItem as a display item that is rendered by aLayer.
>     */
> +  void AddLayerDisplayItem(Layer* aLayer,
> +                           nsDisplayItem* aItem,
> +                           LayerState aLayerState = LAYER_ACTIVE);

Don't use a default parameter here, just pass it explicitly everywhere.
Comment 25 User image Benjamin Stover (:stechz) 2011-05-18 16:09:47 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/9024deba23fe
Comment 26 User image Brad Lassey [:blassey] (use needinfo?) 2011-05-18 20:14:22 PDT
Comment on attachment 533296 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

If I understand our current branch status, this will be merged onto aurora next week automatically because you've landed it on trunk.

I'll approve for beta after a couple days of baking on trunk
Comment 27 User image Asa Dotzler [:asa] 2011-05-18 23:23:26 PDT
What's the potential fall-out for Desktop if we take this into Beta?
Comment 28 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-19 02:02:10 PDT
Extremely low since desktop doesn't create LAYER_ACTIVE_EMPTY layers.
Comment 29 User image Anna (Waverley) 2011-05-19 08:59:03 PDT
I can also see a similar behavior on Fennec 5.0b2 mainly when typing in the username field from login pages.

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:5.0) Gecko/20110517 Firefox/5.0 Fennec/5.0

See screenshot.
Comment 30 User image Anna (Waverley) 2011-05-19 09:00:13 PDT
Created attachment 533662 [details]
Straight lines between characters
Comment 31 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-05-19 09:37:21 PDT
Anna, can you update to the 19th build please to see if you see the same issue?
the 19th build should have fixed it.  I've checked with:

Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110519 Firefox/6.0a1 Fennec/6.0a1
Device: Droid 2 
OS: Android 2.2

Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110519 Firefox/6.0a1 Fennec/6.0a1
Device: Thunderbolt
OS: Android 2.2
Comment 32 User image Anna (Waverley) 2011-05-19 10:00:25 PDT
Yes, it seems to be fixed on the nightly build (on the 19th build).

But, Naoki, isn't it supposed to be fixed on Beta too ? I could see this issue happening quite frequently even though unfortunately I could not reproduce it 100%.
Comment 33 User image Benjamin Stover (:stechz) 2011-05-19 10:40:22 PDT
Anna, this is currently not fixed on Beta. Once this bug is verified for nightly and we have approval, we can push it to the beta branch and make a new beta build.
Comment 34 User image Benjamin Stover (:stechz) 2011-05-19 10:41:17 PDT
Oops, I somehow overwrote VERIFIED!
Comment 35 User image Asa Dotzler [:asa] 2011-05-19 14:53:14 PDT
Comment on attachment 533296 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

Would be nice to have someone from mobile in these triage meetings. Approving because this doesn't impact desktop.
Comment 36 User image Asa Dotzler [:asa] 2011-05-19 15:16:16 PDT
Comment on attachment 533296 [details] [diff] [review]
Password field is not masked immediately and shows artifacts, when typing past the end of the field

Please land this change on both Aurora and Beta. (In the future, getting changes in during Aurora will save you this extra step.)
Comment 37 User image Brad Lassey [:blassey] (use needinfo?) 2011-05-19 15:17:24 PDT
(In reply to comment #35)
> Comment on attachment 533296 [details] [diff] [review] [review]
> Password field is not masked immediately and shows artifacts, when typing
> past the end of the field
As I said in comment 26, I'd like this to bake for a few days. IMO this shouldn't land on beta until next week.

> Would be nice to have someone from mobile in these triage meetings.
> Approving because this doesn't impact desktop.
Where/when was the beta triage call announced? Probably better to answer that on irc or something.
Comment 38 User image Anna (Waverley) 2011-05-23 07:43:24 PDT
VERIFIED FIXED on:

Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110523 Firefox/6.0a1 Fennec/6.0a1 


Mozilla /5.0 (Android;Linux armv7l;rv:5.0a2) Gecko/20110522 Firefox/5.0a2 Fennec/5.0a2 

Device: HTC Desire Z (Android 2.2)
Comment 39 User image christian 2011-05-26 14:01:50 PDT
Did this land on beta? I ask because comment 38 implies it did.
Comment 40 User image Matt Brubeck (:mbrubeck) 2011-05-26 14:07:23 PDT
Yes: http://hg.mozilla.org/releases/mozilla-beta/rev/c04ad1e37383

Note You need to log in before you can comment on or make changes to this bug.