808408 - Hide Virtual Keyboard when bookmarks list is opened

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[wolpi.dev]

Attachment: 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]

Device: HTC Sensation
Android Version: 4.0.3

Comment 2

[Aaron Train [:aaronmt]]

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)]

This used to work. Its a regression.

Comment 4

[Aaron Train [:aaronmt]]

Lucas?

Comment 5

[Lucas Rocha (:lucasr)]

Yeah, some (recent?) change broke focus handling in Awesome Screen. It would probably help to know when this happened.

Comment 6

[wolpi.dev]

I noticed it when I upgraded form FF15 to 16 via google play.

Comment 7

[Adrian Tamas (:AdrianT)]

Possible dupe of 770170?

Comment 8

[Aaron Train [:aaronmt]]

Looks like it

Comment 9

[Aaron Train [:aaronmt]]

Can someone carry over the tracking 18+ to bug 770170.

Comment 10

[Wesley Johnston (:wesj)]

Attachment: Patch

This fixes two bugs (switching screens and scrolling the list).

Comment 11

[Brian Nicholson (:bnicholson)]

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 13

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f48d082d7d0

Comment 14

[Wesley Johnston (:wesj)]

I'm hoping this was the cause of robocop failures. backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4bfe7e32928

Comment 16

[Aaron Train [:aaronmt]]

Was it?

Comment 17

[Brian Nicholson (:bnicholson)]

Also, did you intentionally omit the change suggested in comment 11?

Comment 18

[Wesley Johnston (:wesj)]

Attachment: 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)]

Looks mostly good on try. Waiting for a retrigger.

Comment 20

[Wesley Johnston (:wesj)]

https://tbpl.mozilla.org/?tree=Try&rev=a64d770a8e9e

Comment 21

[Geoff Brown [:gbrown]]

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)]

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)]

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)]

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 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8a94633be564
https://hg.mozilla.org/mozilla-central/rev/3eafe802b846

Comment 26

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ec301dff48c
https://hg.mozilla.org/releases/mozilla-aurora/rev/3cdce9376da6

Comment 27

[Aaron Train [:aaronmt]]

(reminder to set fixed on the channel)

Comment 28

[Ed Morley [:emorley]]

(In reply to Wesley Johnston (:wesj) from comment #26)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/7ec301dff48c
> https://hg.mozilla.org/releases/mozilla-aurora/rev/3cdce9376da6

This broke robocop on aurora:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=3cdce9376da6

Aurora backout:
https://hg.mozilla.org/releases/mozilla-aurora/rev/900485806593

Comment 29

[Lukas Blakk [:lsblakk] use ?needinfo]

Wesley - can you take another look at landing this on Aurora?

Comment 30

[Wesley Johnston (:wesj)]

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]]

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]]

I have a fix for the testBookmark problem coming soon in bug 769919.

Comment 33

[Wesley Johnston (:wesj)]

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)]

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]]

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)]

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