1071114 - Landscape rocketbar shrinking animation is off

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Gregor Wagner [:gwagner]]

I have a debug gecko build which shows an obvious glitch in the shrinking animation for the rocketbar.
When showing the collapsed rocketbar, the URL field starts as a rectangle and gets rounded corners after some time.

Comment 1

[Gregor Wagner [:gwagner]]

[Blocking Requested - why for this release]:

This is also pretty bad with an optimize build.

Comment 2

[Kevin Grandon :kgrandon]

It seems like this is possibly a graphics issue?

Comment 3

[Gregor Wagner [:gwagner]]

Logcat also shows this during the transition:
E/GeckoConsole( 1087): [JavaScript Error: "TypeError: element is undefined" {file: "app://system.gaiamobile.org/js/statusbar.js" line: 589}]

Comment 4

[Kevin Grandon :kgrandon]

(In reply to Gregor Wagner [:gwagner] from comment #3)
> Logcat also shows this during the transition:
> E/GeckoConsole( 1087): [JavaScript Error: "TypeError: element is undefined"
> {file: "app://system.gaiamobile.org/js/statusbar.js" line: 589}]

I think this was a false-positive fixed today in bug 1082667.

Comment 5

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

It might be a platform issue with OMTA. The border radius seems to be ignored for the duration of the transform transition. If I remove the transition:transform at [1] then the corners remain rounded as expected. I did notice also that the border-radius comes from [2] when the urlbar is maximized and a mixture of [2] (for the left corners) and [3] (for the right corners) when the urlbar is minimized, so that might also have something to do with it.

[1] https://github.com/mozilla-b2g/gaia/blob/edbce405a3dc087520df73207e204dc9a8d0f803/apps/system/style/chrome/chrome.css#L198
[2] https://github.com/mozilla-b2g/gaia/blob/edbce405a3dc087520df73207e204dc9a8d0f803/apps/system/style/chrome/chrome.css#L110
[3] https://github.com/mozilla-b2g/gaia/blob/edbce405a3dc087520df73207e204dc9a8d0f803/apps/system/style/chrome/chrome.css#L144

Comment 6

[Kevin Grandon :kgrandon]

It does appear that we have border-radius on two different elements currently. In the long term I'd like to avoid this by using display: box, but that is going to be a major refactoring of rocketbar.

For now - can we move this into the graphics component to see why border-radius is being ignored during OMTA? Kats - is this something you can help us with?

Comment 7

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

Matt might be a better person for this. I'm guessing it's some sort of restrictions with mask layers or something?

Comment 8

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

Brian may be able to help.

Comment 9

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Any chance of a video demonstrating the problem? Based on the description I can't reproduce the problem on a build from about 3 weeks ago.

Comment 10

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

Attachment: Video of the bug

Comment 11

[bhavana bajaj [:bajaj]]

Can QA help test this on a 2.1 production build ?

Comment 12

[Joshua Mitchell (Inactive)]

Issue Repro's on Flame 2.1 and Flame 2.2  (Feature not available in 2.0)

Note - Only occurs in LANDSCAPE mode 

Device: Flame 2.1 - KK base - Shallow Flash
Build ID: 20141021001201
Gaia: e458f5804c0851eb4e93c9eb143fe044988cecda
Gecko: ee86921a986f
Version: 34.0 (2.1)
Firmware Version: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 - KK Base - Shallow Flash
Build ID: 20141021040206
Gaia: 457a54fc3200b80e4f5e1cd3acaa062309230732
Gecko: 29fbfc1b31aa
Version: 36.0a1 (2.2)
Firmware Version: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 13

[Dietrich Ayala (:dietrich)]

Triage decided, sadly, not blocking because landscape mode which reduces user impact, and doesn't really meet any other blocker criteria. If you have a patch landed on master, re-nom once you have a risk estimate, or just ask for patch approval to uplift.

Comment 14

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

CC-ing No-Jun, there are other "landscape only" issues creeping in, wonder if they're related.

Comment 15

[Gregor Wagner [:gwagner]]

There is also another bug with the software home button enabled where the rocketbar in extended state stick around for too long:
https://www.youtube.com/watch?v=2SUEAnvkPvk&feature=youtu.be

Comment 16

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Hi Gregor, sorry I won't be able to look at this properly until next week but can I get a pointer to the source for the animation/transition in landscape mode?

Comment 17

[Kevin Grandon :kgrandon]

Hi Brian. This animation is driven with CSS transitions.

Expanded state: https://github.com/mozilla-b2g/gaia/blob/ac3768b899f2f1d6f31a046f4b28d3f75bcad517/apps/system/style/chrome/chrome.css#L207
Collapsed state: https://github.com/mozilla-b2g/gaia/blob/ac3768b899f2f1d6f31a046f4b28d3f75bcad517/apps/system/style/chrome/chrome.css#L196
Class toggle: https://github.com/mozilla-b2g/gaia/blob/ac3768b899f2f1d6f31a046f4b28d3f75bcad517/apps/system/js/app_chrome.js#L312

Let me know if there's any additional information you need!

Comment 18

[Brian Birtles (:birtles, Feb 26~ parental leave)]

It seems to me we're doing some reflow of the start of this transition that we don't do when we're in portrait mode. I see some flicker at the start of the animation but only in landscape mode. In fact, it seems like we start doing the animation, do some restyling, then restart the animation. Is there any difference to the content when we are in landscape mode?

It seems like we might end up layerizing without the border-radius applied. When we drop the layer at the end of the transition we re-render with the correctly computed border-radius. I'm curious if there's some other action we take when the collapse the rocketbar (particularly if it's something that only happens in landscape mode) that causes a reflow. (Unfortunately I can't get WebIDE to talk to my Flame anymore so I can't easily check this myself yet.)

(Another, less likely, possibility is that we are actually rendering the border-radius but the layer is too small/large so by the time we scale it the border-radius appears to be square.)

Comment 19

[Chris Lord [:cwiiis]]

Adding n?vingtetun for comment #18 - I'm not sure how the controls are hidden, but that may cause a reflow... But I don't think there's any difference between what happens in portrait and landscape.

It might be worth disabling the HWC (hardware compositor) in the dev settings and restarting, just to rule that out.

Comment 20

[Gregor Wagner [:gwagner]]

Kevin or Etienne should be able to help out as well.

Comment 21

[Kevin Grandon :kgrandon]

I'll look into this soon, but I agree - I don't think there's a difference between portrait and landscape. 

(In reply to Chris Lord [:cwiiis] from comment #19)
> It might be worth disabling the HWC (hardware compositor) in the dev
> settings and restarting, just to rule that out.

I've disabled HWC and the issue still reproduces.

Comment 22

[Etienne Segonzac (:etienne)]

Attachment: First try

I think the issue might come from the instant where the background is sampled (??) for the rounded corners.

The dom looks like this
|background div |bigger border-radius div|smaller border-radius div|||

At the very beginning of the transition the color behind the smaller div is the heavy gray from the bigger div, the same color than the div itself, so you can't actually see the radius. Then the background of the bigger div switches to transparent, but we're transitioning a transform on the smaller div at the same time and maybe not re-sampling what's behind the corners?

Anyway, setting a background-color on the bigger deiv instead of having it be transparent fixes the issue.
Also, we were dispatching the appchomecollapsed a bit early since the controls transition was finishing earlier (and bubbling to our transition end listener).

Comment 23

[Kevin Grandon :kgrandon]

Thanks for picking this up Etienne!

Comment 24

[Chris Lord [:cwiiis]]

Comment on attachment 8512777 [details] [review]
First try

Nothing about these changes looks like it'd cause any problems and the event listener stuff looks good. Good work, assuming it works around the bug :)

Comment 25

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR

New cleaned-up version with tests, ready for review!

Comment 26

[Chris Lord [:cwiiis]]

Comment on attachment 8513651 [details] [review]
Gaia PR

Looks good :)

Comment 27

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/0f6aa5d044073b2ec08b21aad48026ae8be57c49

Comment 28

[Kevin Grandon :kgrandon]

This is a dependency of bug 1090562. Uplifted to v2.1: https://github.com/mozilla-b2g/gaia/commit/746e26d5f6298b965e8e95e885884bd6dc82ae67

Comment 29

[Brogan Zumwalt [Inactive]]

Issue verified fixed in Flame 2.1 and Flame 2.2

Actual Results: The Rocketbar has rounded edges at all times. Gregor's shrinking issue in comment 15 does not manifest with or without SHB enabled.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104001202
Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5
Gecko: 388b03efe92d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104040207
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 54d05732f29b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0