Closed Bug 1231554 Opened 4 years ago Closed 4 years ago

[Galaxy Note] Blank strip across bottom of rendered pages

Categories

(Firefox for Android :: Toolbar, defect)

43 Branch
ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed
fennec 45+ ---

People

(Reporter: larrikin, Assigned: kats)

References

Details

(Keywords: regression)

Attachments

(4 files, 2 obsolete files)

A blank rectangle of the same size as the rectangle that would define toolbar & tab-tops appears on the bottom of the screen, animates in mirror to the toolbar & tab-tops. It does not shift hotspot location, so clicks become misaligned with clickable items.

I believed this issue was as reported by #1203058 however testing 43 releases (43b10 latest) subsequent to this fix has not resolved my issue.

Hardware: Samsung GT-N8000, Android: 4.1.2

The problem is not in 42.
When you say "animates in mirror" do you mean that when the URL bar is visible, the blsnk space is visible also, and when the URL bar is hidden, the blank space is also hidden? And for the click offset issue, do you only see that when the blank space is visible? If so it sounds like the LayerView isn't translating as intended. I can make a build with extra logging for you to run and get more information if that is the case.
Yes to all q's.
NI kats to make sure this doesn't get lost – do you want to follow up with the build?
Flags: needinfo?(bugmail.mozilla)
Yup, thanks. I did a try push to [1], once it's done there will be a build in the folder at [2].

Scott, please install and run the build (it will install as "Nightly"), and reproduce the problem while running |adb logcat -v time *:V|. Please save the resulting log and attach it to this bug (or email it to me if you prefer). Thanks!

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad55bbdd057b
[2] http://archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-ad55bbdd057b0270b6cd7e86272d47913306213a/try-android-api-11/
Flags: needinfo?(bugmail.mozilla)
Flags: needinfo?(larrikin)
Duplicate of this bug: 1232893
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
I am having trouble getting a Win 7 PC working with adb. No success getting Kies S/W to recognise the GT-N8000, nor is any device listed with 'adb devices' after enabling USB debugging on the device.  I will need more time.
Flags: needinfo?(larrikin)
Flags: needinfo?(larrikin)
Aaron, according to the device list on the wiki, you used to have a galaxy note - do you still have it? I think all instances of this bug so far have been on some version of the Note.
Flags: needinfo?(aaron.train)
I only have an original Galaxy Note phone. Device in reference in this bug is the semi new tablet.
Flags: needinfo?(aaron.train)
FWIW I also flashed a Galaxy Nexus with 4.1.2 and couldn't reproduce it there. Seems this is indeed specific to the Galaxy Note 10.1.
Summary: Blank strip across bottom of rendered pages → [Galaxy Note] Blank strip across bottom of rendered pages
Attached image Blank rectangle GS5 (obsolete) —
Scott, is this the blank rectangle you're seeing? I reproduced this on my GS5 on http://www.sheldonbrown.com/brakturn.html
Comment on attachment 8701282 [details]
Blank rectangle GS5

(In reply to Michael Comella (:mcomella) from comment #12)
> Scott, is this the blank rectangle you're seeing? I reproduced this on my
> GS5 on http://www.sheldonbrown.com/brakturn.html

Nevermind – I see the same bar in Chrome. Notably, it's an ad bar that, likely because I have tracking protection enabled, does not show ad content in FF but does in Chrome.
Attachment #8701282 - Attachment is obsolete: true
FYI here is a screen-shot of the bug on the Galaxy Note 10.1: https://bugzilla.mozilla.org/attachment.cgi?id=8700285 (from a duplicated bug). The white bar below is what is mentioned in the bug description. The bar's color seems to depend on the background color of the site. Also note, that the top area of the website (mozilla.org in this case) is overlapped by the URL bar and all touch input is vertically offset.

About a device:
I own a Galaxy Note 10.1. Can I be of any help?
Thanks, if you can provide a log as per the instructions in comment 4 that would be helpful!
tracking-fennec: ? → 43+
Assignee: nobody → bugmail.mozilla
Attached file adb log output
Took some time, but here is the logfile. I followed the instructions in comment 4.
Thanks! The logfile confirms that the code is behaving as expected; we're attempting to scroll the ScrollView as the toolbar slides in and out. Unfortunately it's not clear from the log why the scrolling isn't happening - it might be a bug elsewhere in our code or it might be a bug in the Galaxy Note version of Android. I'll make another try build to verify a few more things and see if I can find a workaround here.
Flags: needinfo?(larrikin)
Try push at [1]. The build should end up in the folder at [2] when it's done. Would you please do the same thing with this build? Thanks!

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=885bce47c990
[2] http://archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-885bce47c9906733663836665a55315e28cd452e/try-android-api-11/
Flags: needinfo?(thomas.seifried)
Here you go, another log file from the new build.
Flags: needinfo?(thomas.seifried)
Thanks for the log! Unfortunately as far as I can tell the bug is definitely somewhere in the Android code and there's no obvious way in our code to detect if this is happening. Moving a SurfaceView by scrolling the enclosing ScrollView seems to just not work on this device even though it works everywhere else, as far as I can tell.

At this point I'm going to have to blindly attempt workarounds because without access to the Samsung code changes it's practically impossible to tell what the underlying bug is and how to properly avoid it. My first workaround attempt is to switch to using a TextureView instead of a SurfaceView. This is the recommended approach in Android 4.1 and up, and it's supposed to integrate better with other Android components. I'm hoping that this will solve the problem. If it does, we can enable this for 4.1 only, which will fix the issue on these devices without inflicting much risk on other devices.

Try push at [1], build will appear at [2]. Please give it a try and see if it fixes the problem. If it does, that's great. If it doesn't, I'll have to try something else. There's a possibility that Fennec will fail to start up entirely, or not render stuff properly. If that happens please attach a log, it might be an error in my workaround.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=c08240905e91
[2] archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-c08240905e9192838d9fbe3540c3201f92b0a33e/try-android-api-11/
It works.
But now a tabs content does not update anymore if the virtual keyboard is open. Only when the keyboard is closed, the tab updates to the correct content.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #20)
> Thanks for the log! Unfortunately as far as I can tell the bug is definitely
> somewhere in the Android code and there's no obvious way in our code to
> detect if this is happening. Moving a SurfaceView by scrolling the enclosing
> ScrollView seems to just not work on this device even though it works
> everywhere else, as far as I can tell.
> 
> At this point I'm going to have to blindly attempt workarounds because
> without access to the Samsung code changes it's practically impossible to
> tell what the underlying bug is and how to properly avoid it. My first
> workaround attempt is to switch to using a TextureView instead of a
> SurfaceView. This is the recommended approach in Android 4.1 and up, and
> it's supposed to integrate better with other Android components. I'm hoping
> that this will solve the problem. If it does, we can enable this for 4.1
> only, which will fix the issue on these devices without inflicting much risk
> on other devices.

Unfortunately, I don't think we'll be able to do this. TextureView is not as performant as SurfaceView because it introduces an extra compositing step. With SurfaceView, the Surface is composited directly by the surfaceflinger. Using TextureView, the Surface is drawn by the UI system into the app's surface, which is then composited by surfaceflinger. When I looked at doing this a while back, WebGL took a 50% hit. It's possible that newer Android is doing some fencing magic to avoid serializing rasterization of the two buffers, though, so it could be worth revisiting.
Can we not do it just for API level 16? According to http://developer.android.com/about/dashboards/index.html it has only 10% marketshare. We can probably further restrict it to this Galaxy Note device if necessary. I don't really have any other possible fix for this in mind.
(In reply to thomas.seifried from comment #21)
> It works.
> But now a tabs content does not update anymore if the virtual keyboard is
> open. Only when the keyboard is closed, the tab updates to the correct
> content.

Glad to hear that it works, although I'm not able to reproduce the issue with the virtual keyboard open. I tried just typing in a text field, and the content I was typing appeared fine in the text field. If you are using some other method to reproduce this problem please let me know.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #24)
I will look to find some time to "play" around with that issue. I will come back when I have more info.
Also, can you go into the Android settings, into "About phone" and tell me what it says for "Model number"? I can use that to make sure we only enable TextureView on this device and not others.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #26)
> Also, can you go into the Android settings, into "About phone" and tell me
> what it says for "Model number"? 

It is GT-N8010
In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #24)

I was wrong in my last comment about the tabs. It is not caused by an open virtual keyboard, but it does not work in general anymore. 

Steps to reproduce:
1. Enter url to go to any website
2. Open new tab
3. Enter url in new tab and go to website
4. Wait until website has been loaded. Everything works fine until here.
5. Switch back to first tab -> website of second tab is still visible. 

Notes: 
1. The virtual keyboard sometimes fixes the problem. If a textbox in the second tab has focus and I switch back to the first tab, sometimes (not always) the content of the first tab is rendered correctly again.
2. As long as I do not switch tabs, the rendering works just fine. I can click on links and all things get updated. Only when I switch back to already opened tabs, things brake.
(In reply to thomas.seifried from comment #27)
> It is GT-N8010

Thanks

(In reply to thomas.seifried from comment #28)
> I was wrong in my last comment about the tabs. It is not caused by an open
> virtual keyboard, but it does not work in general anymore. 

Hm, this seems problematic. I can't reproduce it on my device (Nexus 4 running 4.2.2) so it might be specific to 4.2.1 or the Galaxy Note. I tried a bunch of stuff including setting the "Don't keep activities" option which might be relevant here but wasn't able to reproduce this. Unless we can fix this problem this approach is not viable, but I can't think of any other options either. :(
Ok, I came up with another option. Try push at [1] and the build will show up at [2] once it's done. Please give it a shot and let me know if things are still wonky with that one. Thanks!

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2e383c90daf
[2] archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-f2e383c90daf0495b626efeda3a077d7b4e938bb/try-android-api-11/
(for comment 30, if you have some time to try another build)
Flags: needinfo?(thomas.seifried)
Duplicate of this bug: 1239980
(reply to comment #30)
It is back to the old (buggy) behavior.

Maybe the device check is not working correctly? Just for more detail here is all the info about the device and software build I was able to find:
Model number: GT-N8010
Android-Version: 4.1.2
Kernel-Version: 3.0.31-805288
Build-Number: JZO54K.N8010XXUCMK2
Flags: needinfo?(thomas.seifried)
Has anyone tried to browse websites with "Full-screen browsing" being disabled?
Webpages see to be right when "Full-screen browsing" is disabled.

Disable "Full-screen browsing": goto Settings -> Display -> uncheck Full-screen browsing

And this issue still can be reproduced on 44.
Hello Bingqing Li

You said "Has anyone tried to browse websites with "Full-screen browsing" being disabled?
Webpages see to be right when "Full-screen browsing" is disabled.

Disable "Full-screen browsing": go to Settings -> Display -> uncheck Full-screen browsing"



That solves the problem, thank you.
(In reply to Bingqing Li from comment #34)
Can confirm that this solves the issue. Tested it on 43 and the current Beta stream. I could not find that option in the settings of nightly try build from comment #30 though.
Duplicate of this bug: 1239981
Duplicate of this bug: 1239407
This missed the 44 train. Is there a solution we can get for 45?
tracking-fennec: 43+ → ?
None of my attempts at fixing it have produced shippable results yet. I'm out of ideas. Maybe we just force-disable full-screen browsing on this device?
Yes that seems like the best way forward.
tracking-fennec: ? → 45+
Oh, in bug 1232246 the reporter said it was also happening on GT-N5110 so I'll add that to the block.
Now blocking the Note 8.0 as well.
Attachment #8713330 - Attachment is obsolete: true
Attachment #8713330 - Flags: review?(margaret.leibovic)
Attachment #8713337 - Flags: review?(margaret.leibovic)
Comment on attachment 8713337 [details] [diff] [review]
Disable dynamic toolbar on Galaxy Note w/4.1.2

Review of attachment 8713337 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/java/org/mozilla/gecko/Restrictions.java
@@ +138,5 @@
> +        //  GT-N5100, GT-N5110, GT-N5120
> +        return Build.VERSION.SDK_INT == Build.VERSION_CODES.JELLY_BEAN
> +            && (Build.MODEL.startsWith("GT-N80") ||
> +                Build.MODEL.startsWith("GT-N51"));
> +    }

This class was created for restricted profiles, and I'm not sure if it makes sense to include other forms of restrictions in here (i.e. things that we didn't disable for a specific profile).

I think it's fine, but I'm redirecting this review to Sebastian, since he designed this system and might know about some edge case I'm missing.
Attachment #8713337 - Flags: review?(margaret.leibovic) → review?(s.kaspari)
I'm happy to move it to a DynamicToolbar.isForceDisabled function instead; I just put it in Restrictions because that's what the other Preferences were doing.
Comment on attachment 8713337 [details] [diff] [review]
Disable dynamic toolbar on Galaxy Note w/4.1.2

Review of attachment 8713337 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, I agree with Margaret here. Let's move it to its own class, or some generic DeviceRestrictions class. Restrictions is used for profile/account restrictions and the functionality is mirrored by nsIParentalControlsService.idl. The class used to be RestrictedProfiles.java but this was confusing because it actually handles restricted as well as guest profiles. Maybe we should have named it UserRestrictions.
Attachment #8713337 - Flags: review?(s.kaspari) → review+
Thanks for the info. I moved the static function into DynamicToolbar.isForceDisabled but kept everything else pretty much the same.
We'll probably want to uplift this all the way to beta once we can get somebody to verify that it works in nightly.
https://hg.mozilla.org/mozilla-central/rev/61dbb4f18478
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
If people seeing this problem can verify on the next nightly that it no longer happens (and that you can't hide the url bar anymore) that would be great. Once we have verification we can uplift this patch and get it into the 45 release rather than having to wait until the 47 release (which is where it is now). Thanks!
Installed Nightly 47.0a1 on Galaxy Note 10.1 GT-N8013 Android 4.1.2.

After downloading and installing around 4pm CST USA time on 1/31/2016, Nightly also wanted to install an updated version.  Did that update also.

Tried Google, Yahoo, and Bing web page searches.  Could click on returned links ok now.  
Also, no "Full Screen Browsing" check box was showing in Settings anywhere that I could find.

However did notice an annoying different bug.  As it is a Nightly build, maybe you are also working on that.  Even with Settings > Accessibility > Magnify small areas > box unchecked, it seemed that Yahoo and Google search boxes would auto zoom when typing.  Also editing a search item word in the search box would not always show backspacing erases or new letters typed.
Thanks! And yes, the issues you mention at the end of your comment are unrelated and are being worked on. They are specific to Nightly.
Comment on attachment 8713337 [details] [diff] [review]
Disable dynamic toolbar on Galaxy Note w/4.1.2

Approval Request Comment
[Feature/regressing bug #]: bug 1180295
[User impact if declined]: on some devices (galaxy note 8 and galaxy note 10 runnung 4.1.2) hiding the dynamic toolbar causes rendering issues and touches/clicks going to the wrong spot. There have been many bugs filed for this issue (see dupes) and I saw some complaints while browsing the play store reviews as well.
[Describe test coverage new/current, TreeHerder]: tested locally and on nightly
[Risks and why]: the patch disabled the dynamic toolbar behavior on affected devices. The main risk is that the device checks are too broad and might block unaffected devices as well by accident, but if that happens it should not introduce any correctness issues.
[String/UUID change made/needed]: none
Attachment #8713337 - Flags: approval-mozilla-beta?
Attachment #8713337 - Flags: approval-mozilla-aurora?
Comment on attachment 8713337 [details] [diff] [review]
Disable dynamic toolbar on Galaxy Note w/4.1.2

OK to uplift to aurora, has had manual testing on m-c.
Attachment #8713337 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8713337 [details] [diff] [review]
Disable dynamic toolbar on Galaxy Note w/4.1.2

let's fix that regression in beta too.
Should be in 45 beta 3
Attachment #8713337 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
has problems uplifting to beta:

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r bdaf65deb7b3
grafting 326915:bdaf65deb7b3 "Bug 1231554 - Disable the dynamic toolbar on Galaxy Note devices running Android 4.1.2. r=sebastian, a=lizzard"
merging mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(bugmail.mozilla)
This is the patch that I used against my own (git) checkout of beta...it is a git commit, however, and not an HG commit.  Feel free to use it if it helps.
Thanks!
Flags: needinfo?(bugmail.mozilla)
Duplicate of this bug: 1249335
Duplicate of this bug: 1234437
You need to log in before you can comment on or make changes to this bug.