Closed Bug 956075 Opened 8 years ago Closed 7 years ago
Long-pressing at top of page with URL bar hidden triggers URL bar context menu
Galaxy Q running recent nightly. STR: 1. Load a page (e.g. mozilla.org) and scroll down so that the URL bar scrolls off the screen. 2. Position the content so that a link is at the top of the visible area (where the URL would be). On mozilla.org I use the "Firefox different by design" linked image. 3. Long-press on the link Expected: Context menu for link appears (includes options like "Open in new tab") Actual: Context menu for URL bar appears (includes options like "Edit site settings")
8 years ago
OS: Mac OS X → Android
Hardware: x86 → ARM
What device/Android version? Unable to reproduce on a Nexus 5.
Galaxy Q running android 2.3.
(In reply to Kartikaya Gupta (email:firstname.lastname@example.org) from comment #2) > Galaxy Q running android 2.3. Kats - Can you try Aurora and Beta to help narrow down the range?
Latest aurora is affected, 27.0b2 is not.
That makes me suspect bug 768667.
Can you have a look at this?
Yes. Brad is bringing a Galaxy Q next week so I can debug.
Assignee: nobody → wjohnston
Good build 11.23.2013; Bad build 11.24.2013; Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=98aa428a392c&tochange=74ab61b8d0f0 Tested on Samsung Galaxy S (Android 2.3.4)
Confirmed bug 768667.
I can reproduce this bug on a DroidPro running Gingerbread. Do we think this is screen size related or Gingerbread related? Hmm, I can test on my Nexus S running Gingerbread too... Yep, it's Gingerbread related. It happens on my Nexus S too.
This is a WIP that attempts to hide the view flipper when its offscreen. I tried this the other day with no success, but trying it today I noticed some "Updating Views off the UI thread" errors I hadn't seen before. I'm not sure if that's because I'm testing on 4.3 now or if they weren't there on 2.3, but might be worth trying again: http://dl.dropboxusercontent.com/u/72157/hideView.apk If that doesn't work, I'd also like to try actually setting the scroll on the view, or maybe adding a third "empty" view to the view flipper.
(In reply to Wesley Johnston (:wesj) from comment #11) > http://dl.dropboxusercontent.com/u/72157/hideView.apk > > If that doesn't work, I'd also like to try actually setting the scroll on > the view, or maybe adding a third "empty" view to the view flipper. Tested on my Nexus S. Does not fix the problem. Tapping still displays the BrowseerSearch UI. Long pressing displays the URLBar context menu.
(In reply to Wesley Johnston (:wesj) from comment #11) > http://dl.dropboxusercontent.com/u/72157/hideView.apk > ARMv7 build? Crashes on startup on my Galaxy Q
This overrides the ViewFlipper with our own class that ignores touches if the ViewFlipper is hidden. armv7 build at: http://people.mozilla.org/~wjohnston/hideView.apk I still haven't been able to pin down exactly where/how this was broken in GB phones, but the code changed drastically. For instance, 4.4 has these canViewReceivePointerEvents checks: http://androidxref.com/4.2.2_r1/xref/frameworks/base/core/java/android/view/ViewGroup.java#1861 that aren't there anywhere in the 2.3 dispatchTouchEvent code: http://androidxref.com/2.3.7/xref/frameworks/base/core/java/android/view/ViewGroup.java#824 I can't believe that all ViewGroups were this broken in 2.3 though...
Just tested the build on my Nexus S and it works. Also tested on my DroidPro and it works.
This gets rid of the listeners and just uses the touch coordinate directly. Its a bit of a hack in that it assumes our view is at 0,0. For now that's fine here....
Attachment #8366257 - Flags: review?(lucasr.at.mozilla) → review+
Is this on central?
No, it was backed out in https://hg.mozilla.org/integration/fx-team/rev/76b9e0793b76
I'm not sure what's going on, but robocop seems to use a hit test when finding these views (I can't seem to find in their source where that is, but I see dispatchTouchEvent being hit). The touch that's used for that has its rawY set slightly outside the bounds of the element, while getX() returns a value thats in the center of the element. getX/Y are probably better to use here anyway. Using them fixes the test locally for me. Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=82fdaec23a17
Attachment #8369284 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8369284 [details] [diff] [review] Patch Review of attachment 8369284 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: mobile/android/base/widget/GeckoViewFlipper.java @@ +11,5 @@ > import android.graphics.Rect; > import android.os.Build; > import android.view.MotionEvent; > import android.widget.ViewFlipper; > +import android.util.Log; Unrelated, remove.
Attachment #8369284 - Flags: review?(lucasr.at.mozilla) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Comment on attachment 8366257 [details] [diff] [review] Patch v3 [Approval Request Comment] Bug caused by (feature/regressing bug #): actionmode stuff User impact if declined: can't tap top of screen Testing completed (on m-c, etc.): landed on mc this week. seems smooth Risk to taking this patch (and alternatives if risky): Pretty low risk. There aren't any other really good solutions I could come up with, beyond dropping 2.3 support or backing out the whole thing. String or IDL/UUID changes made by this patch: None.
Verified as fixed on builds: - 29.0a2 (2014-02-10); - 30.0a1 (2014-02-10). Device: Samsung Galaxy S (Android 2.3.4).
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.