1149457 - [Message]The recipient will be hidden when we tap the reciepient to edit.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

Description

[Alissa（leave from Mozilla）]

Attachment: logcat2.txt

[1.Description]:
[Nexus 5 v2.2&v3.0][Flame 2.2][Message] Add much recipients in new message, tap the recipient to edit, sometimes,we cannot slide down the recipients input field agian.
Found at:14:28 
see attachments:logcat2.txt, VIDEO0466.3gp

[2.Testing Steps]: 
1.Launch message.
2.Tap edit button.
3.Tap the recipient inut box.
4.Add more than 20 recipients.
5.Tap the recipient in first line.

[3.Expected Result]: 
5.The right editing page should be displayed.

[4.Actual Result]: 
5.You can see the editing number cannot be displayed.

[5.Reproduction build]: 
Device:Nexus5_2.2[Affected]
Build ID               20150330162503
Gaia Revision          cc11248ab69f13e89416c8e6bb2e184187e72088
Gaia Date              2015-03-30 22:22:58
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/90a26917ee8f
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150330.201653
Firmware Date          Mon Mar 30 20:17:07 EDT 2015
Bootloader             HHZ12d

Device:Nexus5_3.0[Affected]
Build ID               20150330160204
Gaia Revision          be25b16efa19bab8d54be08f8fe45dcc93bf93d0
Gaia Date              2015-03-29 10:19:00
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/6dedce1ca673
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150330.191451
Firmware Date          Mon Mar 30 19:15:04 EDT 2015
Bootloader             HHZ12d

Device:Flame 2.2[Affected]
Build ID               20150330162503
Gaia Revision          cc11248ab69f13e89416c8e6bb2e184187e72088
Gaia Date              2015-03-30 22:22:58
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/90a26917ee8f
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150330.201412
Firmware Date          Mon Mar 30 20:14:21 EDT 2015
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
occasionally Recurrence,5/10

[7.TCID]: 
Free Test

Comment 1

[Alissa（leave from Mozilla）]

Attachment: VIDEO0466.3gp

Comment 2

[Julien Wajsberg [:julienw]]

This looks like a Graphics issue: the white block appearing should actually be a dialog.

Milan, can you help diagnosticating?

Comment 3

[Mason Chang [Inactive] [:mchang]]

Both this bug and bug 1149461 are not affected by the vsync refresh driver since we disable it on L. The 3.0 commit for the Nexus 5 L has it disabled.

Does this also affect the Flame on master or only 3.0 on the Nexus 5?

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

And even more explicitly - does it effect Flame on KitKat?

Comment 5

[Alissa（leave from Mozilla）]

Hi mason, this issue also exist on Flame 3.0
Device:flame 3.0
Build ID               20150331160205
Gaia Revision          03164bd160809747e6a198e0dba1b7c3ee7789f5
Gaia Date              2015-03-31 14:48:14
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/18a8ea7c2c62
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150331.191641
Firmware Date          Tue Mar 31 19:16:50 EDT

Comment 6

[Mason Chang [Inactive] [:mchang]]

I can take an initial look at this.

Comment 7

[Mason Chang [Inactive] [:mchang]]

From irc with Botond:

We took at the APZ layers dump and this doesn't look like an APZ bug. The scrollable layer for the recipients disappears from the layer tree when the bug occurs, so APZ has nothing to scroll. This is probably either a layout bug or gaia app bug. Still digging. 

I also suspect this may be very related to bug 1149461.

Comment 8

[Mason Chang [Inactive] [:mchang]]

I went back to the working revision listed here - https://bugzilla.mozilla.org/show_bug.cgi?id=1149461#c7 - This issue still reproduces, so unrelated to bug 1149461.

Comment 9

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer dump on a flame with bug

Comment 10

[Mason Chang [Inactive] [:mchang]]

Attachment: Pic of bug on Flame

The problem is a bit different on a flame device. We still see the unknown contact up top. I am very confused as to why the message area is so big instead of a single line.

Comment 11

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer dump on a flame w/o bug

Comment 12

[Keven Kuo [:kkuo]]

Hi! Mason,

This looks like not Lollipop specific issue. It also happens on flame-kk, right?
Do you agree to remove "Blocks: 1094121"? Thanks

--
Keven

Comment 13

[Mason Chang [Inactive] [:mchang]]

(In reply to Keven Kuo [:kkuo] from comment #12)
> Hi! Mason,
> 
> This looks like not Lollipop specific issue. It also happens on flame-kk,
> right?
> Do you agree to remove "Blocks: 1094121"? Thanks
> 
> --
> Keven

Sure, removed the blocking bug.

Comment 14

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Dump Differences

Narrowed down the layer trees to the differences. In the bad case, we're missing a bunch of layers, including a scrollable ID where we would scroll the recipients list. The pounds are still the same, except in the bad case, the layer is shadow-visible whereas it isn't in the good case. Still digging, although not quite sure what to look for yet.

Comment 15

[Mason Chang [Inactive] [:mchang]]

Attachment: Display list dump in good case

Comment 16

[Mason Chang [Inactive] [:mchang]]

Attachment: Display list dump in bad case

Comment 17

[Mason Chang [Inactive] [:mchang]]

Attachment: Annotated display list dump - good version

Comment 18

[Mason Chang [Inactive] [:mchang]]

Attachment: Annotated display list dump - bad version

A few things that are interesting.

1) The nsDisplayTransform that wraps the recipient list does not get a layer in the bad version.
2) The layer that contains the back button and "24 recipients" does not exist in the bad version.
3) Each recipient in the good version contains 3 display items: background color, text, and LayerEventRegion. The bad version has these 3 items plus the LayerEventRegion is a child of a WrapList. We get 24 extra WrapList items in the bad version.
4) The display item to render the "To:" field is gone in the bad version.

So essentially it looks like for some reason, the "To field" and above are gone, causing things to disappear.

I also noticed that the problem mostly occurs when we need to create a new line in the recipients field. Everytime we tap an old recipient, it moves the recipient from the top of the list to the bottom of the list. If the targeted recipient can fit on the existing "to" line, it's fine. If the targeted recipient must be layed out on a new line because it cannot fit on the current line, that's when we hit the problem.

Comment 19

[Mason Chang [Inactive] [:mchang]]

Attachment: Frame tree in bad version

Comment 20

[Mason Chang [Inactive] [:mchang]]

Attachment: Frame tree in good version

Comment 21

[Mason Chang [Inactive] [:mchang]]

A couple of things strike me as odd.

1) Everytime we tap on a new icon, the SMS app actually clears the recipients list and re-adds the HTML to show the list. It also changes focus a couple of times on different items in the list. I think that this also tells the keyboard app to hide, then show up again, just most of the time we don't see the keyboard animation hide/come back up. Sometimes, you can see the keyboard animate out then back in. This is because we focus on the tapped element which should cause the keyboard to come back up.
2) The messages input data looks like it has a variable height, even though in the WebIDE, the actual height of the messages input field is the same in the good version / bad version. You can see this in the video. At first, after tapping an item, the Messages input section looks large, then when the items are scrolled into view, the messages input section becomes the normal size. If the list is small enough, the keyboard / messages size always stay the same and scrolling the list just covers the contacts area.
3) The recipients list while typing the list sometimes doesn't update the correct number of recipients. 

Right now I'm thinking there is some race condition with the input focus event, detecting/animating if the keyboard is up or not, and layout reflow. I think I'm going to focus on figuring out why the messages area has lots of weird whitespace. I wonder if we're reflowing while the keyboard animation is coming up and not waiting until the keyboard has finished loading, therefore causing bad sizes of everything. Just a theory though, still kind of digging in the dark.

Comment 22

[Mason Chang [Inactive] [:mchang]]

BTW Julian, sometimes I see a dialog that says "Would you like to remove this contact" and sometimes the keyboard shows up to edit the contract. From comment 2, when should it actually be a dialog?

Comment 23

[Mason Chang [Inactive] [:mchang]]

I'm still pretty lost, so if someone wants to chime in feel free. What I have found is that we have lots of threads and animations occurring at the same time. 

When we scroll the recipients list, we set the whole list to a 'multiline' CSS class and have a transitionend listener. When we tap on an item in the list, we do a couple of things. We clear the whole list, rebuild the html for the recipients list, and put the tapped recipient at the end of the list and set focus on the last item. During this time, I THINK the keyboard tries to animate out since it no longer has focus, but I can't find anywhere in gecko or gaia where the events "input-appopened" fire. I do however see focus/blur events being fired in gecko during this transition. 

The SMS app also listens to the 'click' event, which focuses on the recipient that was clicked and renders the view, based on the scrolltop (https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/recipients.js#L618), which scrolls to the end of the list. I think reading ScrollTop forces a layout flush, but I have to double check (https://dxr.mozilla.org/mozilla-central/source/dom/base/Element.cpp?from=ScrollTop#754). While all this is happening, we're rendering the view with the keyboard possibly animating in/out. What I'm confused about is why sometimes the keyboard actually does finish rendering out, then back in and sometimes doesn't change at all. When it does render out/in, the issue goes away.

What's also interesting is that sometimes, the keyboard gets the blur event and sometimes it doesn't, which I think somewhere in the keyboard app or somewhere, we perhaps cancel or stop the blur event fired in gecko somewhere in gaia. But I don't really know how this happens or why.

Another thing I've observed is that during this transition, we actually render the view twice. When the scrolltop from the first refocus event is larger than the second scrolltop read during the focus, the issue doesn't appear. It only appears when the scrolltop from the first focus event is smaller than the second focus event. So I think we're reading scrollTop and rendering in the SMS App before the keyboard animations / state is actually finished due to all the blur / focus events. 

Julien, do you know what the proper sequence for the keyboard animation / SMS rendering should be? Thanks!

Comment 24

[Julien Wajsberg [:julienw]]

(In reply to Mason Chang [:mchang] from comment #23)
> I'm still pretty lost, so if someone wants to chime in feel free. What I
> have found is that we have lots of threads and animations occurring at the
> same time. 
> 
> When we scroll the recipients list, we set the whole list to a 'multiline'
> CSS class and have a transitionend listener. When we tap on an item in the
> list, we do a couple of things. We clear the whole list, rebuild the html
> for the recipients list, and put the tapped recipient at the end of the list
> and set focus on the last item. During this time, I THINK the keyboard tries
> to animate out since it no longer has focus, but I can't find anywhere in
> gecko or gaia where the events "input-appopened" fire. I do however see
> focus/blur events being fired in gecko during this transition. 


So, this is tricky because the event name is the result of a concatenation, as you can see at [1].

"eventPrefix" is set to "input-app" in input_window.js [2]; InputWindow "inherits" from AppWindow.

[1] https://github.com/mozilla-b2g/gaia/blob/af29b7755788ef03414298a3e298a936aefcbf53/apps/system/js/app_window.js#L1358
[2] https://github.com/mozilla-b2g/gaia/blob/af29b7755788ef03414298a3e298a936aefcbf53/apps/system/js/input_window.js#L63

Now I don't really know how all this works. NI John Lu who can help with all the questions about the Keyboard.

> 
> The SMS app also listens to the 'click' event, which focuses on the
> recipient that was clicked and renders the view, based on the scrolltop
> (https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/recipients.
> js#L618), which scrolls to the end of the list. I think reading ScrollTop
> forces a layout flush, but I have to double check
> (https://dxr.mozilla.org/mozilla-central/source/dom/base/Element.
> cpp?from=ScrollTop#754). While all this is happening, we're rendering the
> view with the keyboard possibly animating in/out. What I'm confused about is
> why sometimes the keyboard actually does finish rendering out, then back in
> and sometimes doesn't change at all. When it does render out/in, the issue
> goes away.
> 
> What's also interesting is that sometimes, the keyboard gets the blur event
> and sometimes it doesn't, which I think somewhere in the keyboard app or
> somewhere, we perhaps cancel or stop the blur event fired in gecko somewhere
> in gaia. But I don't really know how this happens or why.
> 
> Another thing I've observed is that during this transition, we actually
> render the view twice. When the scrolltop from the first refocus event is
> larger than the second scrolltop read during the focus, the issue doesn't
> appear. It only appears when the scrolltop from the first focus event is
> smaller than the second focus event. So I think we're reading scrollTop and
> rendering in the SMS App before the keyboard animations / state is actually
> finished due to all the blur / focus events. 
> 
> Julien, do you know what the proper sequence for the keyboard animation /
> SMS rendering should be? Thanks!

I'm not sure a proper sequence has really been specified.
I can only agree all this part is really complex.

Let's see if John can shed a little more light :)

Comment 25

[Julien Wajsberg [:julienw]]

(In reply to Mason Chang [:mchang] from comment #22)
> BTW Julian, sometimes I see a dialog that says "Would you like to remove
> this contact" and sometimes the keyboard shows up to edit the contract. From
> comment 2, when should it actually be a dialog?

Sorry, forgot about this question.

The dialog appears when you tap a recipient that is a contact. But a contact means either a known contact (from your Contacts app) or an unknown contact (means someone you already sent/received a message from, if you used the suggestions list entry to add it).

Comment 26

[John Lu [:mnjul][Please NI or I'll miss]]

I'm not exactly sure how I can help on this but here are some insights & event sequences:

1. Normal keyboard showing
- the keyboard InputWindow state is currently |closed|.
- we get |inputmethod-contextchange| with some input context at KeyboardManager [1], which eventually calls InputWindowManager#showInputWindow [2] and InputWindow#open [3].
- Keyboard app prepares it self and after it's done, we get |mozbrowserresize| event at [4]. At this very moment, the InputWindow state is still |closed| and it's not visible. Then, still at this tick, we trigger the ready message (|input-appready|) and calls AppWindow#open [5].
- (At this moment, you get |input-appopening|. But we're not currently listening to it.)
- AppTransitionController listens to the |animationend| event [6]. When the animation ends, you get |input-appopened|, as in [7].
- (For completion's sake) At this moment, InputWindowManager sends |keyboardchange| event to other System app components [8].

2. Normal keyboard hiding
- the keyboard InputWindow state is currently |opened|.
- we get |inputmethod-contextchange| with |type === blur| at [1], which calls InputWindowManager#hideInputWindow [9] and InputWindow#close [10]. The latter calls AppWindow#close. Similarly to the case above, we get |input-appclosing| when the slide-out animation is going to begin, and |input-appclosed| when the animation finishes.
- (Again for completion's sake) At |input-appclosing|, InputWindowManager send |keyboardhide| event to other System app components; at |input-appclosed|, it sends |keyboardhidden|.

3. Re-showing keyboard when it's transitioning out
- So, when the InputWindow state is |closing|, we get |inputmethod-contextchange| with some input context. Note that at this moment InputWindowManager._currentWindow is null.
-  The entire reshowing logic is carried out by a special logic at InputWindow#open [11] which simulates the "ready" message and also dictates the keyboard is to be immediately open (bypassing animation).
-  Note that the "immediate" logics reside in AppTransitionController.
- You get |input-appopened| directly, no |input-appopening|.

4. Re-hiding keyboard when it's transitioning in
We actually almost go through the normal route as case 2; AppTransitionController [12] dictates that if an AppWindow is |opening| and we get a close() call, we transition to closing.

Bonus point: you can actually programmatically force keyboard to show/hide without animation. It's currently already possible with hiding through InputWindowManager#hideInputWindowImmediately [13]. I believe similar should be achievable with showing, but I have never tried that, though.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/keyboard_manager.js#L216

[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window_manager.js#L397

[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L216

[4] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L94

[5] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L128-L130

[6] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_transition_controller.js#L381

[7] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_transition_controller.js#L116-L141

[8] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window_manager.js#L167

[9] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window_manager.js#L428

[10] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L206

[11] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L238-L248

[12] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_transition_controller.js#L12

[13] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window_manager.js#L439

Comment 27

[Julien Wajsberg [:julienw]]

In case this has an impact, note that Bug 1149461 just landed on master.

Comment 28

[Julien Wajsberg [:julienw]]

In case this is not clear: when the keyboard is displayed, the application is resized; same when the keyboard is hidden.

I'm not sure exactly when the resize happens in the steps outlined by John in comment 26.

Comment 29

[John Lu [:mnjul][Please NI or I'll miss]]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #28)
> In case this is not clear: when the keyboard is displayed, the application
> is resized; same when the keyboard is hidden.
> 
> I'm not sure exactly when the resize happens in the steps outlined by John
> in comment 26.

Ah! In that case: the resizing happens when LayoutManager receives |keyboardchange| or |keyboardhide| [1][2]. These events are dispatched in the 4 cases in comment 26 as we encounter |input-appopened| and |input-appclosing|, at InputWindowManager.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/layout_manager.js#L150
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/layout_manager.js#L179

Comment 30

[Mason Chang [Inactive] [:mchang]]

So after digging some more, I'm not really sure this is a graphics / layout bug. I'm fairly new to gaia and digging through from gaia -> layout takes a while so I may have the wrong analysis, but this seems more like a Gaia bug.

When I tap a recipient in the list, we get the click event. Sometimes, this forces a focus at [1], which causes an update to render. We also set the recipients list to css 'singleline' which has an animation we listen to the 'animationend' [2]. During this time, we also bring up the keyboard, which sometimes is immediate [3] and sometimes not. If it is not immediate, then I think we should be waiting for the keyboard to animate out, but I do not see the keyboard animate out and I rarely see the call to [3] occurring. I also sometimes see that we render the inner HTML view twice after a click, which I'm not sure why, which causes different scrollHeights in the inner view. The height as determined by the keyboard from the LayoutManager [4] is always correct. 

After the app_window_manager fires the system-resize event, it starts to resize() the Messages App, but we can sometimes have the transitionend occur after we resize().

I think this is more of a gaia / keyboard / system app issue. Seems like we should either not render the recipients list until after the keyboard has finished animating out to come back in, or wait to show update the recipients list set the view until after the keyboard resize event has been fired. What do you think Julien?

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/recipients.js#L357
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/recipients.js#L698
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/input_window.js#L238-L248
[4] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/layout_manager.js#L159

Comment 31

[Mason Chang [Inactive] [:mchang]]

On a side note from layout's side, in the display list, the nsDisplayTransform that contains all the recipients gets a layer in the good version and doesn't get one in the bad version. 

Good version - nsDisplayTransform p=0xb10ea888
Bad version - nsDisplayTransform p=0xb10e95a0

This causes all the children to be in a WrapList, see comment 18. Do wraplist / something not getting a layer with nsDisplayTransforms ring any bells for you Timothy? Or anything I should be checking for to see if this is a layout issue? Thanks!

Comment 32

[Timothy Nikkel (:tnikkel)]

Normally we shouldn't get a wraplist in an after optimization (as it looks like in the bad dump) dump because we flatten them away here:

http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#3483

Are we calling ContainerState::ProcessDisplayItems for that display transform at all? It doesn't seem like it. If not, why not? FrameLayerBuilder::BuildContainerLayerFor calls ProcessDisplayItems, you should be able to use aContainerFrame or aContainerItem to isolate the call for the transform item you care about.

Comment 33

[Julien Wajsberg [:julienw]]

(In reply to Mason Chang [:mchang] from comment #30)

> I think this is more of a gaia / keyboard / system app issue. Seems like we
> should either not render the recipients list until after the keyboard has
> finished animating out to come back in, or wait to show update the
> recipients list set the view until after the keyboard resize event has been
> fired. What do you think Julien?

The piece of code that handles the recipient is overly complex and I definitely want to eventually simplify this. But because this wasn't broken it's quite low priority.

My main issue here is: at one moment, the situation is stable (nothing moves), and we have a wrong display.
If there is any glitch between the moment the user taps and this stable situation, we can look at this after. But the stable situation itself is wrong.

What do you think? Is it easier to debug this stable situation?

Comment 34

[Mason Chang [Inactive] [:mchang]]

Attachment: display list - bad w/ screenshot

Comment 35

[Mason Chang [Inactive] [:mchang]]

Attachment: screenshot bad w/ attachment 8590521

Comment 36

[Mason Chang [Inactive] [:mchang]]

Attachment: Bad Display Dump

Comment 37

[Mason Chang [Inactive] [:mchang]]

Attachment: Bad screenshot

Comment 38

[Mason Chang [Inactive] [:mchang]]

Attachment: Good display dump

Comment 39

[Mason Chang [Inactive] [:mchang]]

Attachment: Good screenshot

Here is a good / bad display list dump and screenshot on the same content. This makes it a lot easier to analyze as the layers should be the same. The original problem of the nsDisplayTransform not optimizing those layers away no longer happens. I confirmed that the nsDisplayTransform is being optimized and going through ContainerState::ProcessDisplayItems.

What's interesting is in the bad version, layer 0xb1275c00, corresponding to nsDisplayTransform (good 0xb193e650, bad 0xb193edd8) has a transform of -83. I think these are screenpixels. 

The layer with the "Message" is actually still the same on both the good/bad version.

The biggest difference is that in the bad version, we have a ClientPaintedLayer (0xb1fcf800) that has content with the "To:" field and background color of white. In the good version, layer 0xb1a64800 is just opaque and optimized out.

Timothy, do you see anything odd in the good / bad versions that stand out to you as to why we would have a box of white underneath "Messages". 

Side note: If I add 30 recipients, this is pretty much permafail. The amount of whitespace grows with the recipients list, so I wonder if we're hitting some kind of limit of the SMS View / flexbox.

Comment 40

[Mason Chang [Inactive] [:mchang]]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #33)
> (In reply to Mason Chang [:mchang] from comment #30)
> 
> > I think this is more of a gaia / keyboard / system app issue. Seems like we
> > should either not render the recipients list until after the keyboard has
> > finished animating out to come back in, or wait to show update the
> > recipients list set the view until after the keyboard resize event has been
> > fired. What do you think Julien?
> 
> The piece of code that handles the recipient is overly complex and I
> definitely want to eventually simplify this. But because this wasn't broken
> it's quite low priority.
> 
> My main issue here is: at one moment, the situation is stable (nothing
> moves), and we have a wrong display.
> If there is any glitch between the moment the user taps and this stable
> situation, we can look at this after. But the stable situation itself is
> wrong.
> 
> What do you think? Is it easier to debug this stable situation?

I don't quite understand what you mean by the stable situation? As in, the end result is wrong? I don't know if it is easier to debug this situation, I'm pretty unfamiliar with both the gaia side and layout side. It seems to me that we shouldn't be calling Recipients.view.Render(). Render() resets the view, changes some CSS depending on scrollHeight, which doesn't seem accurate until the keyboard has finished loading and until the app_window.resize() occurs. I am mis-understanding what all this code does since I haven't done a lot of JS, but I still think it's an SMS problem in that we're stuck because we rendered before the resize().

Comment 41

[Timothy Nikkel (:tnikkel)]

(In reply to Mason Chang [:mchang] from comment #39)
> What's interesting is in the bad version, layer 0xb1275c00, corresponding to
> nsDisplayTransform (good 0xb193e650, bad 0xb193edd8) has a transform of -83.
> I think these are screenpixels. 
> 
> The layer with the "Message" is actually still the same on both the good/bad
> version.

I think indicates we are painting the content given to us correctly. The layer with the -83 translate is an ancestor of the layer with the "Message" text so that's why it is rendering higher up. The display list dump has the same transform beside the display transform item and it's quite unlikely this debug info is computed incorrectly, it's likely coming directly from the style set on the frame. So this makes a strong case that it's not layout/gfx.

Comment 42

[Julien Wajsberg [:julienw]]

OK Mason, I think I got your point this time.

I'll look closer.

But I see we didn't even do a branch check; I somehow assumed it was a regression because I never saw the issue. So let's do that first.

Comment 43

[Julien Wajsberg [:julienw]]

Also, there is clearly several issues; Mason I think you investigated the issue where the general layout seemed to be off.

But the original reported issue is: the "remove this recipient" dialog displays as a white rectangle.

While the first issue could be a Gaia::SMS issue, the second one is definitely a gfx or layout issue...

Comment 44

[Mason Chang [Inactive] [:mchang]]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #43)
> Also, there is clearly several issues; Mason I think you investigated the
> issue where the general layout seemed to be off.
> 
> But the original reported issue is: the "remove this recipient" dialog
> displays as a white rectangle.
> 
> While the first issue could be a Gaia::SMS issue, the second one is
> definitely a gfx or layout issue...

I thought the remove this recipient dialog was bug 1149461, or the fact that you cannot see the list of recipients due to the SMS issue. Or is there a different issue you are talking about?

If you add enough recipients, you can no longer see the recipient list due to this SMS issue. I am interpreting that "white rectangle" as the same issue.

The bug where tapping on a recipient is sometimes blank or black is bug 1149461.

Can you please assign yourself to this bug? Thanks!

Comment 45

[Brogan Zumwalt [Inactive]]

Issue does reproduce in Flame 2.0 and 2.1

Scrolling up in recipient list with 30 or more numbers listed then tapping to edit results in list and other elements being hidden until focus is changed to another element.

Device: Flame 2.1
Build ID: 20150410001204
Gaia: bbe983b4e8bebfec26b3726b79568a22d667223c
Gecko: 81a3aad1e6d7
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 46

[Julien Wajsberg [:julienw]]

Moving components for now, keeping the NI, I want to test some things, especially related to bug 1149461.

Renominating 2.2 given this exists for several versions.

Comment 47

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:

Comment 48

[Julien Wajsberg [:julienw]]

So I don't reproduce the issue reported initially, and I rarely reproduce the issue that Mason investigated. It's difficult to reproduce for me but I saw it at least 3 times while testing.

I think there is a race somewhere as Mason said. I'm not sure this is because of scrollHeight.

Could QA help finding a somewhat reliable and short STR to reproduce the situation in attachment 8590550 [details] ? It's a mix of having a lot of recipients, sliding down/up the recipient panel, changing focus between different parts of the window, typing recipients, using the suggestions list.

Thanks !

Comment 49

[Pi Wei Cheng [:piwei] (inactive)]

There are too many things going on in this bug. I believe comment 45 reproduced the original bug described on comment 0, and I'm still able to repro it on today's 3.0. Anything other than the original bug should be filed separately. And from a screenshot (attachment 8590550 [details]) it's really hard to tell what's going on, such as what's wrong with it and if it's wrong then what's the expected behavior.

Comment 50

[Julien Wajsberg [:julienw]]

It's weird, I have a hard time reproducing this, even focusing on what happens in attachment 8590550 [details].

Pi Wei, can you please share your precise STR to reproduce what happens in this attachment?

Comment 51

[Pi Wei Cheng [:piwei] (inactive)]

I'm confused. I don't think we ever reproduced attachment 8590550 [details]. We reproduced the bug where tapping on the first recipient the whole UI goes pretty much blank (like shown in the video of comment 1, and described on comment 45).

I don't know what's going on with attachment 8590550 [details] so I'm afraid I can't help you there.

Comment 52

[Julien Wajsberg [:julienw]]

Pi Wei, I don't reproduce this either. I mean never. Can you please share your STR?

Comment 53

[Pi Wei Cheng [:piwei] (inactive)]

The STR is basically putting a bunch of numbers separated by commas and then scroll up to tap on the first recipient. I did a video demonstrating the issue from scratch:

https://www.youtube.com/watch?v=EkheB7bx6hA

(Please bear with me keep hitting the wrong key in the video as holding the phone in an unusual position makes me think I am tapping on a place that it's not)

Comment 54

[Julien Wajsberg [:julienw]]

Thanks, got it, you need a phone number. It doesn't reproduce with contact recipients.

Now I reproduce very easily:

1. you need to put enough recipients that the recipient panel needs to scroll when it's open.
2. it needs to be open and scrollable. (it does not matter if the keyboard is displayed or not). The more scrollable it is, the more serious the problem appears.
3. you need to "tap to edit" the phone number recipient. Before tapping, it doesn't matter where the focus is, but we can have the problem appearing differently depending where the focus was before.

Comment 55

[Julien Wajsberg [:julienw]]

1.bis: you need to put at least one phone recipient. It can be anywhere in the list.

Comment 56

[Pi Wei Cheng [:piwei] (inactive)]

I supposed qawanted is no longer needed since developers can now repro the bug. Removing keyword.

Comment 57

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 58

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(