1109315 - [Email] Folder list will be shown when navigating through account settings

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Chris Kreinbring [:CKreinbring]]

Description:
As the user navigates through the settings for an email account, the user will see the folder list as the setting pages slide in and out.
   
Repro Steps:
1) Update a Flame device to BuildID: 20141209040946
2) Launch the Email app.
3) Log in with a valid username and password.
4) Tap the drawer icon then the settings gear icon.
5) Tap either the email account or the Add Account button and observe the screen as the appropriate page loads.
  
Actual:
As the pages slide in and out of the screen, the list of email folders will be briefly visible.
  
Expected: 
The settings pages will slide in and out of the screen with nothing shown underneath except for the previous settings page.
  
Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141209040946
Gaia: 9e0b96c7b61c7ff943876ca93e2596d972437b80
Gecko: 47f0671e2c65
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
  
Repro frequency: 100%
See attached video clip

Comment 1

[Chris Kreinbring [:CKreinbring]]

The bug repros on Flame 2.1 engineering on shallow flash.
Actual result: As the pages slide in and out of the screen, the list of email folders will be briefly visible.

Flame 2.1
BuildID: 20141209084847
Gaia: 155424d51cf9bb2ea41dc6667f94741f82f35bc0
Gecko: fe5329227c54
Platform Version: 34.0
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

--------------------------------------------------------------------------------------------------------

The bug does not repro on Flame 2.0 engineering with shallow flash.
Actual result: The pages move without anything shown underneath during the transition.

Flame 2.0
BuildID: 20141208061237
Gaia: 856863962362030174bae4e03d59c3ebbc182473
Gecko: 2d0860bd0225
Platform Version: 32.0
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Joshua Mitchell (Inactive)]

Blocking 2.2: too late to block on 2.1 (I think) so will nom for 2.2 instead. Regression, poor UX, visibly bad

Comment 3

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20140827212446
Gaia: 3a838afca295c9db32e1a3ec76d49fb7fe7fd2d2
Gecko: 3be45b58fc47
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First broken
BuildID: 20140828040749
Gaia: 39cad6c82122b964f12a66771bfbcc14fb342d9e
Gecko: 2a15dc07ddaa
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Working Gaia / Broken Gecko = Repro
Gaia: 3a838afca295c9db32e1a3ec76d49fb7fe7fd2d2
Gecko: 2a15dc07ddaa
Broken Gaia / Working Gecko = No repro
Gaia: 39cad6c82122b964f12a66771bfbcc14fb342d9e
Gecko: 3be45b58fc47
Gecko pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3be45b58fc47&tochange=2a15dc07ddaa


B2G Inbound
Last working
BuildID: 20140827174748
Gaia: 5fe336a0650b8cf68e43eb1aa5903c570e814b73
Gecko: 8de94f23bb9f
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First broken
BuildID: 20140827191449
Gaia: 5fe336a0650b8cf68e43eb1aa5903c570e814b73
Gecko: 4a7c80b9ae9c
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Working Gaia / Broken Gecko = Repro
Gaia: 5fe336a0650b8cf68e43eb1aa5903c570e814b73
Gecko: 4a7c80b9ae9c
Broken Gaia / Working Gecko = No repro
Gaia: 5fe336a0650b8cf68e43eb1aa5903c570e814b73
Gecko: 8de94f23bb9f
Gecko pushlog: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=8de94f23bb9f&tochange=4a7c80b9ae9c

Comment 4

[Joshua Mitchell (Inactive)]

Caused by the patch to  Bug 1055760 - can you take a look Kats?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I'll try to get around to looking at this this week. If not I'll definitely get to it by nearly next week. Leaving needinfo on me for now.

Comment 6

[James Burke [:jrburke]]

Switching component for better tracking, feel free to switch it back if there is guidance for something the email app should do to address it.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

[Blocking Requested - why for this release]:
Changing from 2.2? to 2.1? because this is actually pretty ugly-looking and (according to previous comments) affects 2.1. Release drivers can mark it 2.2+ if they don't want to mark it 2.1+

Comment 8

[Kartikaya Gupta (email:[e-mail])]

So this is pretty easy to reproduce but it's much easier to see what's happening if you slow down the transition animation by changing the duration to 10s at [1]. What you see is that the header is fully painted and transitions over fine, but the content area is unpainted so while the transition is happening you can see stuff underneath. You can force a repaint in the middle of the transition by touching the screen (for example) and that will repaint the part of the incoming content that is visible, but the rest of it will still be unpainted. If this were APZ-related I would guess that the displayport is too small but since this is OMTA I'm not so sure. If it is a bug in the platform, it's likely in the layout code. However it might just be expected behaviour given the CSS we're dealing with. Removing the overflow-x:hidden at [2] for example makes the issue go away. My knowledge of overflow isn't that good but it's possible that the part of the card that's offscreen is considered overflow-x and so doesn't get painted.

[1] https://github.com/mozilla-b2g/gaia/blob/ad5580b8e6529e5eebf8517026286d0a8350d5fd/apps/email/style/common.css#L68
[2] https://github.com/mozilla-b2g/gaia/blob/ad5580b8e6529e5eebf8517026286d0a8350d5fd/apps/email/style/common.css#L179

Comment 9

[bhavana bajaj [:bajaj]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #7)
> [Blocking Requested - why for this release]:
> Changing from 2.2? to 2.1? because this is actually pretty ugly-looking and
> (according to previous comments) affects 2.1. Release drivers can mark it
> 2.2+ if they don't want to mark it 2.1+

I checked the video Chris has and its a nasty looking regression that we cannot ship with. Jet per :kats comment above, can we have have someone from layout team look at this?

Comment 10

[Jet Villegas (inactive)]

(In reply to bhavana bajaj [:bajaj] from comment #9)
> I checked the video Chris has and its a nasty looking regression that we
> cannot ship with. Jet per :kats comment above, can we have have someone from
> layout team look at this?

We've got a known regression point with the fix for bug 1055760. We've also got a workaround suggested in comment 8 (Removing the overflow-x:hidden)

We can pull the regressing patch or add the CSS fix in the Gaia code. I'd vote to fix the CSS in Gaia as suggested.

Comment 11

[Johnny Stenback (:jst)]

:jrburke, what do you think of doing the CSS workaround here as suggested in comment 8? The alternative seems to be to back out the offending patch, but that's a non-trivial patch that's been in the tree for 4 months at this point. I'd guess there's considerable risk in backing out that patch.

Comment 12

[James Burke [:jrburke]]

I tried the following on latest master, flame-kk pvt build, shallow gaia+gecko flash on top of v188 base image:

Removing the overflow-x did not seem to help the issue to my eyes. Here is a zip of the email app that has it removed for the .scrollregion-below-header section, as mentioned in Comment 8's [2] link:

http://jrburke.com/work/gaia/email-overflow.zip

Unzip the contents so that they are in an email-overflow directory, and it can be installed with the Web IDE on a flame device.

In general, I am hesitant to modify the overflow-x, as we use that to make sure there is no horizontal, but to allow an overflow-y scroll, so that we have an always-visible header, with a vertical scroll-only area below it.

However, I did notice that removing the `will-change: scroll-position` for that same selector seemed to fix the problem. Zip with just that change (keeps the overflow-x):

http://jrburke.com/work/gaia/email-will-change.zip

(unzip to a email-will-change directory to install via Web IDE)

As I recall, we mostly wanted the will-change speedup for the message list (and probably message reader) scrolling, so if this seems like the right workaround, I can prep a fix that removes will-change for this more generic selector used by most cards to get a scrollable area below a header, and provide a special CSS class for the .scrollregion-below-header sections that will benefit the most from the will-change value.

I would like to confirm this is the correct workaround to use though first, so asking Kats.

(I am on PTO until Jan 5, but can prep a fix if needed before then, ni? me if so)

Comment 13

[Kartikaya Gupta (email:[e-mail])]

If it works that sounds like a fine workaround to me.

Comment 16

[James Burke [:jrburke]]

:jet, in light of comment 12, where overflow-x did not seem to fix the issue for me when tested on master, could the platform issue be something around the interaction between will-change and the transform that animates the card? The regression window finds a commit that leads to the bug, but instead of a backout, perhaps it just uncovered a new interaction with will-change?

Apologies if I targeted the ni? incorrectly.

As for a workaround in the email app, while I could apply it, and have prepped a pull request here:

https://github.com/mozilla-b2g/gaia/pull/27135

However, I would like to confirm that the core issue was found, and perhaps a spin-off bug if appropriate can be tracked for a fix. I expect we could hit it again at some point if it is a will-change/transform combo that does it. 

The workaround works out because where we really need will-change, scrolling the email list, just happens to be the bottom of the div stack and is not transform-slided in, but that could change if the UX changes.

Note for historical tracking, will-change was added to these email elements as part of bug 945461.

I created a standalone zip file of a webapp that shows the issue, if that helps in debugging:

http://jrburke.com/work/gaia/willchange.zip

Unzip the .zip file and the WebIDE can be used to install on a flame device. Tapping "Add Account" should show a black region where the will-change:scroll-position is applied to the .scrollregion-below-header. If that style is removed, the "black region while transform animates" goes away. The style/mail.css file can be edited at the top of the file to toggle the setting of that style.

Comment 17

[Jet Villegas (inactive)]

(In reply to James Burke [:jrburke] from comment #16)
> :jet, in light of comment 12, where overflow-x did not seem to fix the issue

OK, thanks for confirming.

> I created a standalone zip file of a webapp that shows the issue, if that
> helps in debugging:
> 
> http://jrburke.com/work/gaia/willchange.zip

Thanks! That should help a lot. I'll ping you if we need more info.

Comment 18

[Jet Villegas (inactive)]

To jwatt for a look.

Comment 19

[Jonathan Watt [:jwatt]]

(In reply to James Burke [:jrburke] from comment #16)
> I created a standalone zip file of a webapp that shows the issue, if that
> helps in debugging:
> 
> http://jrburke.com/work/gaia/willchange.zip

Thanks for doing that; usually that would be very helpful. Unfortunately in this case that reduced app seems not to reproduce the issue for me so I spent a lot of time trying to repro using it without success, leading me to think maybe the issue has been fixed.

To provide more detail, I first tried to repro on the desktop (where debugging in a lot easier). I couldn't reproduce this using the zip in the 2.1 or 2.2 simulators, either in Firefox 34 (what 2.1 seems to be build on), 35, 36/37 (what 2.2 seems to be build on) or 38.

I also tried to repro using the zip on a Flame device with various 2.1, 2.2 and master images without success. Initially this was with the base image that has a value for ro.bootloader of L1TC300110F0. I then updated it to 188 and tried the various images again. Still no repro.

I would have tried with the actual E-mail app earlier, but an incompatibility with something my mail server is doing prevented me creating an E-mail app account. (A lot of trial and error eventually led me to narrow down the issue and file bug 1121455.)

Once I figured out how to work around the E-mail app issue I did manage to reproduce in 20141209040946 (a 2.2 build, and the ID reported in comment 0) and in latest 2.1 on a Flame device. I am not able to reproduce in latest 2.2 however, so I _think_ this is only a 2.1 issue now (confirmation from someone else welcome!). I'm also not able to reproduce using the E-mail app in the simulator using any of the permutations mentioned previously.

One of the times that I discussed this bug with jrburke on IRC he mentioned that he'd seen the error:

  "Will-change memory consumption is too high. Surface area covers 323840 pixels, budget is the document surface area multiplied by 3 (96960 pixels). All occurences of will-change in the document are ignored when over budget."

I'm not seeing that though in the 2.1 latest build that I can repro on. The logcat output I get is as follows:

When tapping the folder picker menu at the top left I get:

  I/GeckoDump( 1411): LOG: pushCard for type: folder_picker

When tapping the gear at the bottom of the window I get:

  I/GeckoDump( 1411): LOG: pushCard for type: settings_main
  I/GeckoDump( 1411): LOG: Preloading cards: setup_account_info,settings_account

When tapping on my account name to enter its settings:

  I/GeckoDump( 1411): LOG: pushCard for type: settings_account

On tapping the back arrow I get:

  I/GeckoDump( 1411): LOG: Preloading cards: setup_account_info,settings_account

Finally, on tapping "Done" I get no output.

Nothing seems off there, although logging of card popping would be nice.

I've been trying to cut down the E-mail app to create my own reduced testcase app that repros, but that's taking more time than is reasonable, and even once there it would probably take a long time to find the bug from frame/layer dumps.

I think at this point the best course of action if we want a 2.1 fix is to bisect to the revision that fixed the issue for 2.2 and then consider uplifting that. I'm not sure who the appropriate FxOS QA people are, so can someone on the FxOS team find someone from QA to do the bisect?

Comment 20

[Jet Villegas (inactive)]

James: jwatt reports UTR on 2.2 Master (please confirm.) Thanks!

Comment 21

[James Burke [:jrburke]]

Correct, I was not able to reproduce with a master flame-kk full image flash that I did yesterday. It is unclear what changed to fix it, the email app has not had changes in this area.

Just now I tried the latest 2.2 flame-kk full image flash from pvt, mozilla-b2g37_v2_2-flame-kk-eng/latest/, and it is fixed there. I tried latest 2.1, mozilla-b2g34_v2_1-flame-kk-eng/latest, and it is still an issue there, was able to reproduce.

Asking for bisection to find the changeset that fixed the issue on master, using the keyword I have seen QA use before, and flagging :PBylenga to be sure I did that correctly for fxos bisections. My rough feeling is that the change that fixed it could be in the week before Monday January 12, as the 12th was the 2.2 branch day.

Comment 22

[Peter Bylenga [:PBylenga]]

Yup that's correct.  Perhaps we should make a "fixedwindow-wanted" keyword for situations like this?

Comment 23

[KTucker [:KTucker][Inactive 3/4/2016]]

b2g-inbound regression window:

First Working Environmental Variables:

Environmental Variables:
Device: Flame 2.2
Build ID: 20150109123130
Gaia: 2c7d14040149e1f9b1bb3972ff150be0472fa6b6
Gecko: da03d6aff37d
Version: 37.0a1 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Last Broken Environmental Variables:

Device: Flame 2.2
BuildID: 20150109085501
Gaia: d102cc0a7a1f346531553bec64588eea9e4594eb
Gecko: 78f910177388
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Working Gaia & Last Broken Gecko - issue DOES repro
Gaia: 2c7d14040149e1f9b1bb3972ff150be0472fa6b6
Gecko:78f910177388

First Working Gecko & Last Broken Gaia - issue does NOT repro
Gaia: 2c7d14040149e1f9b1bb3972ff150be0472fa6b6
Gecko:da03d6aff37d

Gecko Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=78f910177388&tochange=da03d6aff37d

Caused by bug 1112332

Comment 24

[Peter Bylenga [:PBylenga]]

James, looks like the patch for bug 1112332 fixed this for master.

Comment 25

[James Burke [:jrburke]]

Thanks Peter! Then I am going to resolve this bug as fixed, by the patch for bug 1112332. I am hesitant to mark this bug as a dupe as it may just be fixed as a side effect of the bug 1112332 changes.

If wanting for fxos 2.1 uplift, it is probably best to discuss on bug 1112332 if it can be uplifted.

Comment 26

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame v2.2
See attachment: verify_video

Flame 2.2 build:
Gaia-Rev        0518f4581a0925c0b703d730ef289ab15cbd1216
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c6aa604a7967
Build-ID        20150125002503
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150125.035924
FW-Date         Sun Jan 25 03:59:36 EST 2015
Bootloader      L1TC000118D0

Comment 27

[Sue]

This bug has been verified fail on latest build of Flame v2.1 and Flame v2.2, I will open a new bug to tracing it.

Flame 2.1 build:
Gaia-Rev        63f291df9b9ad8498fb8fc7fb8bf070524406a5c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/70b8982a523d
Build-ID        20150202001432
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150202.035318
FW-Date         Mon Feb  2 03:53:29 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 build:
Gaia-Rev        d6141fa3208f224393269e17c39d1fe53b7e6a05
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/be206fa2fb60
Build-ID        20150202002507
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150202.035604
FW-Date         Mon Feb  2 03:56:15 EST 2015
Bootloader      L1TC000118D0

Comment 28

[Sue]

I have opened a new bug, see Bug 1128810