970428 - [B2G][Browser] Black and white bar appears on bottom when scrolling or zooming on webpage

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect)

Description

[Brogan Zumwalt [Inactive]]

Attachment: Screenshot - Non-Mobile Page

Description: 
When on web browser a black and white bar appears on bottom of screen. This bar appears on mobile versions of webpages most often when scrolling down, scrolling up to top of page causes bar to disappear. When on non-mobile versions of webpages zooming in can cause this bar to appear, zooming back out causes bar to disappear.

Repro Steps:
1) Updated Buri to BuildID: 20140210004002
2) Open Browser app
3) Navigate to mobile webpage where scrolling vertically is possible (i.e. weww.breakingnews.com)
4) Scroll down

Actual:
Black and white bar appears when using browser.

Expected:
No visual glitches appear when using browser.

Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20140210004002
Gaia: 5c8416fb1ea4a27f172ee6386ab3c19135448506
Gecko: 9c9382f433c0
Version: 28.0
Firmware Version: V1.2-device.cfg

Notes:
Repro frequency: 3/3, 100%
See attached: screenshot, youtube video link - http://youtu.be/BfS2AbWyZuo

Comment 1

[Brogan Zumwalt [Inactive]]

Issue also occurs in 1.4

Result: 
Black and white bar appears when using browser.

Environmental Variables:
Device: Buri v1.4 Mozilla RIL
BuildID: 20140210040202
Gaia: c273bd6525f7f295539592ce74d5e6b225d53be1
Gecko: ecf20a2484b6
Version: 30.0a1
Firmware Version: V1.2-device.cfg

Comment 2

[Jason Smith [:jsmith]]

Does this reproduce on 1.1?

Comment 3

[Roland Kunkel [:RolandK]]

(In reply to Jason Smith [:jsmith] from comment #2)
> Does this reproduce on 1.1?

I was unable to reproduce the issue in the latest v1.1 build. Scrolling down on several different websites did not yield the same bar seen in the video and screenshot.

Environmental Variables:
Device: Buri 1.1 MOZ
BuildID: 20140210041201
Gaia: c5cb252e5d1aa45d14f3a2ea182e73e2271e4496
Gecko: c29d165756ab
Version: 18.0
Firmware Version: V1.2-device.cfg

Comment 4

[Jason Smith [:jsmith]]

I can reproduce as well. Yuck.

Comment 5

[Preeti Raghunath(:Preeti)]

Milan

Please review

Comment 6

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

Are we sure that bug 966510 didn't fix this? Jason, do you remember what build you reproduced it on in comment 4? The one in comment 3 is before bug 966510 landed, and I'm not seeing this on my 1.3 build with recent code.

Comment 7

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

I'll rebuild with bug 966510 reverted to check.

Comment 8

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

Ok turns out I can still reproduce it on latest 1.3. I'll take a look.

Comment 9

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

Also needinfo to benfrancis in case it's obvious to him what's happening. Screenshot in comment 0. It only happens the very first time you scroll the toolbar off, as far as I can tell.

Comment 10

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

It looks like the "expanded" class name doesn't get properly added to the attribute the first time around.

Comment 11

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

This is a browser app bug. On startup, showAddressBar() gets called while this.addressBarState is null. This means that the transitionend listener to remove the 'expanded' classname gets registered, but since no transition actually happens, that listener doesn't get removed.

Later, when hideAddressBar() gets called for the first time, it hides the address bar and runs both transitionend listeners, which ends up removing the 'expanded' classname.

Comment 12

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

Attachment: Patch

This fixes the problem, but I'm not sure if it's the "correct" fix. Probably better to not call this function on startup at all but I haven't looked into where that happens. Please feel free to steal this patch.

Comment 13

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

I don't think a window is needed anymore.

Comment 14

[Ben Francis [:benfrancis]]

Comment on attachment 8375693 [details] [diff] [review]
Patch

This patch looks fine fine.

It might be neater to just initially set addressBarState to VISIBLE on line 45.

No idea why we haven't come across this before now.

Comment 15

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

(In reply to Ben Francis [:benfrancis] from comment #14)
> It might be neater to just initially set addressBarState to VISIBLE on line
> 45.

I tried both VISIBLE (ReferenceError: VISIBLE is not defined) and this.VISIBLE (also didn't work) so I'm leaving the patch as-is.

Pull request at https://github.com/mozilla-b2g/gaia/pull/16260 - could you merge it please? (I don't have gaia push access).

Comment 16

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

Comment on attachment 8375693 [details] [diff] [review]
Patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): unknown
[User impact] if declined: the first time the address bar is scrolled off in the browser the content is not resized properly and you can see ugly stuff underneath (see screenshot)
[Testing completed]: locally
[Risk to taking this patch] (and alternatives if risky): pretty low-risk, there's not much that can go wrong here i think. affects gaia only.
[String changes made]: none

Comment 17

[Gregor Wagner [:gwagner]]

https://github.com/mozilla-b2g/gaia/commit/ade88673d00414c3177f7444543b2fa01324708e

Comment 18

[bhavana bajaj [:bajaj]]

Comment on attachment 8375693 [details] [diff] [review]
Patch

low risk , null check. Safe to take on 1.3. Approving.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v1.3: c2b2325af8beaeee1efbaba3cd276d931909af8f