Last Comment Bug 808408 - Hide Virtual Keyboard when bookmarks list is opened
: Hide Virtual Keyboard when bookmarks list is opened
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: Awesomescreen (show other bugs)
: 16 Branch
: ARM Android
: -- normal (vote)
: Firefox 20
Assigned To: Wesley Johnston (:wesj)
:
Mentors:
: 770170 813018 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-04 08:25 PST by wolpi.dev
Modified: 2016-07-29 14:30 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
affected
verified
verified
20+
20+


Attachments
screenshot_bookmarks_keyboard.png (86.58 KB, image/png)
2012-11-04 08:25 PST, wolpi.dev
no flags Details
Patch (1.92 KB, patch)
2012-11-15 14:05 PST, Wesley Johnston (:wesj)
bnicholson: review+
Details | Diff | Splinter Review
Patch (5.57 KB, patch)
2012-12-11 15:01 PST, Wesley Johnston (:wesj)
gbrown: review+
Details | Diff | Splinter Review

Description wolpi.dev 2012-11-04 08:25:59 PST
Created attachment 678128 [details]
screenshot_bookmarks_keyboard.png

User Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

- open firefox
- rotate device to landscape mode
- tap url bar to open awesomescreen
- tap bookmarks


Actual results:

- Virtual Keyboard is shown, so I cannot see my bookmarks. Have to hide Virtual Keyboard myself.


Expected results:

- Virtual Keyboard should be hidden so that I can see my bookmarks
Comment 1 wolpi.dev 2012-11-04 08:27:50 PST
Device: HTC Sensation
Android Version: 4.0.3
Comment 2 Aaron Train [:aaronmt] 2012-11-04 10:01:36 PST
Ideally the URL bar should lose focus. It seems to do so when you switch between Top Sites and Bookmarks back and forth, so I wonder why the first time the focus is kept.

CC :ibarlow, :mfinkle for input here
Comment 3 Wesley Johnston (:wesj) 2012-11-04 10:19:43 PST
This used to work. Its a regression.
Comment 4 Aaron Train [:aaronmt] 2012-11-05 06:44:21 PST
Lucas?
Comment 5 Lucas Rocha (:lucasr) 2012-11-06 03:46:00 PST
Yeah, some (recent?) change broke focus handling in Awesome Screen. It would probably help to know when this happened.
Comment 6 wolpi.dev 2012-11-07 13:25:31 PST
I noticed it when I upgraded form FF15 to 16 via google play.
Comment 7 Adrian Tamas (:AdrianT) 2012-11-13 07:03:51 PST
Possible dupe of 770170?
Comment 8 Aaron Train [:aaronmt] 2012-11-13 07:07:42 PST
Looks like it

*** This bug has been marked as a duplicate of bug 770170 ***
Comment 9 Aaron Train [:aaronmt] 2012-11-13 07:09:05 PST
Can someone carry over the tracking 18+ to bug 770170.
Comment 10 Wesley Johnston (:wesj) 2012-11-15 14:05:32 PST
Created attachment 682180 [details] [diff] [review]
Patch

This fixes two bugs (switching screens and scrolling the list).
Comment 11 Brian Nicholson (:bnicholson) 2012-11-15 15:09:54 PST
Comment on attachment 682180 [details] [diff] [review]
Patch

>-                if (event.getAction() == MotionEvent.ACTION_DOWN)
>+                if ((event.getAction() & MotionEvent.ACTION_MASK) == MotionEvent.ACTION_DOWN)

It'd probably be cleaner to use MotionEvent.getActionMasked() to apply the action mask.
Comment 12 Kevin Brosnan [:kbrosnan] 2012-11-15 15:42:39 PST
*** Bug 770170 has been marked as a duplicate of this bug. ***
Comment 14 Wesley Johnston (:wesj) 2012-11-15 21:01:54 PST
I'm hoping this was the cause of robocop failures. backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4bfe7e32928
Comment 15 Aaron Train [:aaronmt] 2012-11-19 07:47:34 PST
*** Bug 813018 has been marked as a duplicate of this bug. ***
Comment 16 Aaron Train [:aaronmt] 2012-11-19 07:48:01 PST
Was it?
Comment 17 Brian Nicholson (:bnicholson) 2012-11-19 13:55:37 PST
Also, did you intentionally omit the change suggested in comment 11?
Comment 18 Wesley Johnston (:wesj) 2012-12-11 15:01:05 PST
Created attachment 691097 [details] [diff] [review]
Patch

Fix the test failures. I apparently added tests to make sure we didn't fix this at one point.

I also put in a fix for finding the bookmarks button in the menu on ICS+ devices (where it no longer has a text label so clickText("Bookmark") doesn't work anymore. Maybe we should move that somewhere more general?
Comment 19 Wesley Johnston (:wesj) 2012-12-11 15:02:02 PST
Looks mostly good on try. Waiting for a retrigger.
Comment 20 Wesley Johnston (:wesj) 2012-12-11 15:02:13 PST
https://tbpl.mozilla.org/?tree=Try&rev=a64d770a8e9e
Comment 21 Geoff Brown [:gbrown] 2012-12-11 15:47:35 PST
Comment on attachment 691097 [details] [diff] [review]
Patch

I may have added that vkb code when the tests started failing -- oops! Glad we are on the same page now. 

Try run looks solid.
Comment 22 Wesley Johnston (:wesj) 2012-12-11 15:54:02 PST
Pushed again. Fingers crossed. I'll request Aurora approval once they're green.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a94633be564
https://hg.mozilla.org/integration/mozilla-inbound/rev/3eafe802b846
Comment 23 Wesley Johnston (:wesj) 2012-12-11 20:43:59 PST
Comment on attachment 682180 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 759041
User impact if declined: impossible to hide the keyboard and see the whole awesomescreen
Testing completed (on m-c, etc.): Landed on mc 12-11
Risk to taking this patch (and alternatives if risky): This is pretty low risk. Hooking up some function I accidentally unhooked.
String or UUID changes made by this patch:  None.
Comment 24 Wesley Johnston (:wesj) 2012-12-11 20:44:43 PST
Comment on attachment 691097 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Same
User impact if declined: Non. Fixes tests
Testing completed (on m-c, etc.): Landed on mc 12-11. Tests now pass
Risk to taking this patch (and alternatives if risky): Low risk. Test only.
String or UUID changes made by this patch: none.
Comment 27 Aaron Train [:aaronmt] 2012-12-12 17:11:58 PST
(reminder to set fixed on the channel)
Comment 29 Lukas Blakk [:lsblakk] use ?needinfo 2012-12-17 16:36:57 PST
Wesley - can you take another look at landing this on Aurora?
Comment 30 Wesley Johnston (:wesj) 2012-12-18 08:27:16 PST
Missed one test change when unbitrotting last time. Pushed again:

https://hg.mozilla.org/releases/mozilla-aurora/rev/5d9eae38d5eb
https://hg.mozilla.org/releases/mozilla-aurora/rev/0bb84e1c8ad6
Comment 31 Ryan VanderMeulen [:RyanVM] 2012-12-18 13:50:52 PST
Still broke robocop. Also, remember that Aurora isn't inbound. You are expected to watch your pushes and back them out if they break things.
https://hg.mozilla.org/releases/mozilla-aurora/rev/c946b3643904

https://tbpl.mozilla.org/php/getParsedLog.php?id=18061313&tree=Mozilla-Aurora

0 INFO SimpleTest START
1 INFO TEST-START | testBookmark
2 INFO TEST-PASS | testBookmark | checking that bookmarks list exists - bookmarks list exists
3 INFO TEST-PASS | testBookmark | bookmarks list has 4 children (the default bookmarks) - 4 should equal 4
Exception caught during test!
java.lang.NullPointerException
	at org.mozilla.fennec_aurora.tests.testBookmark.runAwesomeScreenTest(testBookmark.java:106)
	at org.mozilla.fennec_aurora.tests.testBookmark.testBookmark(testBookmark.java:55)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:521)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
	at org.mozilla.fennec_aurora.tests.BaseTest.runTest(BaseTest.java:120)
	at junit.framework.TestCase.runBare(TestCase.java:127)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:118)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
4 INFO TEST-UNEXPECTED-FAIL | testBookmark | Exception caught - java.lang.NullPointerException
5 INFO TEST-END | testBookmark | finished in 42833ms
6 INFO TEST-START | Shutdown
7 INFO Passed: 2
8 INFO Failed: 1
9 INFO Todo: 0
10 INFO SimpleTest FINISHED
Comment 32 Geoff Brown [:gbrown] 2012-12-23 19:45:24 PST
I have a fix for the testBookmark problem coming soon in bug 769919.
Comment 33 Wesley Johnston (:wesj) 2013-01-09 14:37:15 PST
This slipped my mind. I fixed up geoff's patch and pushed it with these to try (based on top of beta):
https://tbpl.mozilla.org/?tree=Try&rev=c707a67cf7e6
Comment 34 Adrian Tamas (:AdrianT) 2013-01-11 02:08:56 PST
Virtual keyboard is hidden when accessing the Bookmarks tab either by tapping on the Tab title or by swipe on Aurora 20.0a2 2013-01-10 on the HTC Desire Z (Android 2.3.3) marking as verified on FF 20 but leaving th bug open for the Robocop test fixes
Comment 35 Alex Keybl [:akeybl] 2013-01-14 12:38:23 PST
Given comment 6, we won't track for release. Feel free to nominate for uplift if you believe the fix is low risk.
Comment 36 Cristian Nicolae (:xti) 2013-01-30 08:08:44 PST
This issue doesn't occur anymore on the latest Nightly. Closing bug as verified fixed on:

Firefox for Android
Version: 21.0a1 (2013-01-29)
Device: Galaxy R
OS: Android 2.3.4

Note You need to log in before you can comment on or make changes to this bug.