Created attachment 660762 [details] logs Firefox Mobile 16.0b3 Device: Samsung Galaxy Tab 2 7.0 (Android 4.0.4)/ HTC Desire Z (Android 2.3.3)/Samsung Galaxy R (Android 2.3.4) Steps to reproduce: 1. Open in different tabs planet.mozilla.org, cnn.com, metacafe.com and a few other pages - 3 to 5 tabs should be ok 2. Go to the planet.mozilla.org and search for "mozilla" 3. Use the down arrow to go to the next match until you reach the last match Expected results: After the last match the page is scrolled to the top and the search begins again Actual results: At some point instead of moving to the next match the app starts to cycle through the open tabs. This is reproducible on the Samsung Tab only in Landscape mode for some reason and works ok in Portrait mode but on the HTC Desire Z and Samsung Galaxy R I was able to reproduce the issue in Portrait mode. Notes: Please see the videocaptures: http://youtu.be/0R9Q2DYjidA, http://youtu.be/cKYLMsM1r5s Not seeing anything in the logs but I am also attaching them. Issue is not reproducible on Firefox Mobile 15.0.1
Margaret - Can you get this to happen?
Trying this on the Galaxy Tab 2 7.0 noted above and I was able to reproduce. I rapidly tapped the next down arrow button and my current tab swapped from Planet Mozilla over to CNN, and then over to MetaCafe. Adrian, can you narrow down a regression-window?
Is this only happening on tablets? I don't have a tablet with me, so I wouldn't be able to debug this until later next week if that's the case.
(In reply to Margaret Leibovic [:margaret] from comment #3) > Is this only happening on tablets? I don't have a tablet with me, so I > wouldn't be able to debug this until later next week if that's the case. Yes; just ran the same test on my Galaxy Nexus and it happened too.
> Yes; just ran the same test on my Galaxy Nexus and it happened too. s/Yes/No
I was also able to reproduce on my Galaxy Nexus, but it didn't always happen. I wonder if this is a regression from bug 771997. I'll investigate further.
Looking at the code, I'm not sure how we're ending up switching tabs. I'm also having trouble reliably reproducing this. I saw it happen once, but not again. Aaron or adrian, can you help me drill down minimal STR?
I can reproduce the issue about 60+% off the time following the steps: 1. Load news.google.com, cnn.com, support.mozilla.org in different tabs 2. Go to the news.google.com and search for a term that does not appear very often like "riot", "news", "cnn" 3. Tap repeatedly on the down arrow. Doing this ~ 15-20 times should change the tab to support.mozilla.org
Tried to reproduce the issue on Nightly 16.0a1 2012-07-11 which has the fix for bug 771997 and I am not able to reproduce the issue. One thing I am seeing is that on that build the keyboard is dismissed on the first match. Issue may be produced by that bug and the fixes for bug 760087 and bug 771989. I will investigate further trying to narrow down a regression range
(In reply to adrian tamas from comment #9) > Tried to reproduce the issue on Nightly 16.0a1 2012-07-11 which has the fix > for bug 771997 and I am not able to reproduce the issue. One thing I am > seeing is that on that build the keyboard is dismissed on the first match. > Issue may be produced by that bug and the fixes for bug 760087 and bug > 771989. I will investigate further trying to narrow down a regression range Sorry I forgot to mention that I can only reproduce the issue if the virtual keyboard is opened. If the keyboard is closed I could not find a way to reproduce the issue.
Please disregard comment 9. Due to the fact that this only happens when the vkb is opened it is very hard to reproduce the bug on older builds before the fix for Bug 771989 was integrated. I believe I have narrowed down a regression range: Good build: 2012-07-07 Bad build: 2012-07-08 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9abaedb956a1&tochange=1751d97cc9e4 I have seen a few more details that need to happen for the issue to be reproducible: - the keyboard needs to be opened with find in page - there has to be a match for the search in one of the other tabs
Sounds like a race condition of events due to the fact that you have to navigate rather quickly in order to reproduce. Due to that, I would lessen the severity of this bug.
Actually I just found the correct STR of the bug. Here are a set of simple steps to reproduce: 1. Load any 3 pages in 3 different tabs 2. Go to the first tab and open the Find in page 3. Tap in the search field to open the keyboard 4. Tap between the 2 up and down arrows Every time you tab between the 2 arrows the tab is changed. I have confirmed this by using the stylus of the Galaxy Note (Android 4.0.4) for precision. I can also confirm that the regression range should be correct as I am unable to reproduce the issue on the 2012-07-07 build. Sorry about about the other comments and presumptions but it seems that while tapping on the down arrow sometimes I would tap between them reproducing the issue and the rest was just some unfortunate coincidences. The issue has nothing to do with content or specific pages the only requirement is for the vkb to be opened with the Find in page bar and in case of the tablet for the device to be in landscape - not sure why the issue is not reproducible in portrait.
Since this has an assignee and clear STR we'll go track this, Margaret do you need any more from QA to get to the bottom of this?
I haven't been able to reproduce the tab switching with the up/down arrows, but I've found that hitting the "x" on the find in page bar will sometimes close a background tab (I've only been able to reproduce that when I've had at least 3 tabs). There's definitely something bad going on here. It seems like our message passing is getting screwed up somehow. I will continue to investigate.
Aha, I found that the tab closing issue was caused by a call to Tabs.closeTab() from inside TabsTray.animateTo: http://hg.mozilla.org/mozilla-central/annotate/6a2d1a3556b9/mobile/android/base/TabsTray.java#l312 There's a similar call to Tabs.selectTab() from inside TabsTray.onTouchEnd, so I suspect that's what's getting called and causing the odd tab switching as well. I'm not familiar with this part of the TabsTray code. Sriram, do you have any idea how this code could be executing after tapping buttons in the find in page bar? I added some logging and found that FindInPageBar's onClick handler is never even being called, so maybe something else is managing to capture our click events? It seems like the only relevant changeset in the regression range is: http://hg.mozilla.org/mozilla-central/rev/8dee2a85c0c9 Perhaps changing the layout around changed the way clicks are getting handled? Any help would be appreciated!
Continuing my investigation, I found that what's happening is that the close button click listener is being fired: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/TabsTray.java#159
I also verified that the selectTab call is coming from TabsTray.onTouchEnd. It seems the root problem here is that TabsTray is getting these click events. I'm having a tough time following the logic behind hiding the tabs tray, but it seems like maybe this is a bad side effect of the way we have our layout set up for animations. I'm starting to think I really need Sriram's help here.
Created attachment 663528 [details] [diff] [review] proof of concept fix It seems like actually hiding the panel instead of just changing its dimensions fixes this issue. However, this patch feels hacky. I feel like it would be nicer to incorporate this in TabsPanel's show/hide methods, but we need to wait for the animations to end. Sriram, do you know a better way of doing this? And does it make sense that the visibility could affect whether or not touch events are captured?
Created attachment 664608 [details] [diff] [review] simpler fix I found that the problem was actually that when a click would miss hitting the button directly (like when you're going really fast and rushing), it would get captured by the TabsTray view below the find in page bar. Capturing these clicks and eating them seems to fix the problem. Build here for QA to verify: http://people.mozilla.org/~mleibovic/tmp/findinpage.apk
(In reply to Margaret Leibovic [:margaret] from comment #20) > Build here for QA to verify: > http://people.mozilla.org/~mleibovic/tmp/findinpage.apk Took a run through this with the same approach in comment #2, and wasn't able to reproduce. >+import android.util.Log; >+ Remove?
(In reply to Aaron Train [:aaronmt] from comment #21) > (In reply to Margaret Leibovic [:margaret] from comment #20) > > Build here for QA to verify: > > http://people.mozilla.org/~mleibovic/tmp/findinpage.apk > > Took a run through this with the same approach in comment #2, and wasn't > able to reproduce. > > >+import android.util.Log; > >+ > > Remove? Oops, yeah :)
Comment on attachment 664608 [details] [diff] [review] simpler fix >diff --git a/mobile/android/base/FindInPageBar.java b/mobile/android/base/FindInPageBar.java >+import android.util.Log; Yeah, looks like it's not needed
Comment on attachment 664608 [details] [diff] [review] simpler fix [Approval Request Comment] Bug caused by (feature/regressing bug #): regression caused by a combination of changes to the tabs/find in page bar layouts User impact if declined: tapping on the find in page bar can cause tabs to unexpectedly switch or close Testing completed (on m-c, etc.): just landed on inbound Risk to taking this patch (and alternatives if risky): low-risk fix to prevent clicks on the find in page bar from getting captured by other views String or UUID changes made by this patch: n/a
Comment on attachment 664608 [details] [diff] [review] simpler fix [Triage Comment] Fix looks very low risk. once green on m-i, please land on m-a and m-b.
Verified on Firefox Mobile 16.0b5 on Samsung Galaxy R running Android 2.3.4. I am no longer able to change the tabs by tapping between the up and down arrows of the "Find in page" bar.