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 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 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 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 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 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 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 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 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 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 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 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 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 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 Robert O'Callahan (:roc) (Exited; 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 Robert O'Callahan (:roc) (Exited; 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 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 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 Robert O'Callahan (:roc) (Exited; 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 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 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 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 Robert O'Callahan (:roc) (Exited; 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 Robert O'Callahan (:roc) (Exited; 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 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 Robert O'Callahan (:roc) (Exited; 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 Benjamin Stover (:stechz) 2011-05-18 16:09:47 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/9024deba23fe
Comment 26 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 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 Robert O'Callahan (:roc) (Exited; 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 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 Anna (Waverley) 2011-05-19 09:00:13 PDT
Created attachment 533662 [details]
Straight lines between characters
Comment 31 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 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 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 Benjamin Stover (:stechz) 2011-05-19 10:41:17 PDT
Oops, I somehow overwrote VERIFIED!
Comment 35 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 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 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 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 christian 2011-05-26 14:01:50 PDT
Did this land on beta? I ask because comment 38 implies it did.
Comment 40 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.