880650 - VKB is not dismissed when switching awesomebar sections after adding a page to reading list

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Build: Firefox for Android 24.0a1 (2013-06-06)
Device: LG Nexus 4 
OS: Android 4.2.2

Steps to Reproduce:
1. Open Firefox
2. Go to nytimes.com and open any article
3. Tap on the Reader Mode icon on URL Bar
4. Tap on Add to Reading List button
5. Tap on Reading List button
6. Change between Bookmarks/Top Sites/History sections.

Expected:
- The VKB is dismissed when taping the History/Top Sites section.

Actual:
- The VKB is not dismissed

Comment 1

[Mark Capella [:capella]]

Looks like it was caused by bug 743158 ...

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

I don't think the VKB should pop up at all if we are showing the reading list. But I was unable to track down which code is showing the VKB. Saving to look at later.

Comment 3

[Mark Capella [:capella]]

I read this all as, 

Behaviour prior to bug 732158

Situation #1
 Tap into Awesombar
  Title is Focused
  Keyboard is Open
 Pan Left
  Title is Focused
  Keyboard is Closed

Situation #2
 Tap in Reading List
  No title to focus
  Keyboard is Open
 Pan Left
  No title to focus
  Keyboard is Closed

After bug 732158

Situation #1
 Tap into Awesomebar
  Title is Focused
  Keyboard is Open
 Pan Left
  Title LOSES Focus   <--- The change they wanted
  Keyboard is closed

Situation #2
 Tap into Reading List
  No title to focus
  Keyboard is Open
 Pan Left
  No title to focus
  Key Stays Open      <--- A change they didn't ask for and this bug is about

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

That's correct. But in addition to that, I was arguing that maybe the keyboard should not be open at all when showing the reading list, so the final scenario would become,

Situation #2
 Tap into Reading List
  No title to focus
  Keyboard is Closed
 Pan Left
  No title to focus
  Key Stays Closed

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 880650 - Clean up hiding/showing VKB in the awesome screen; r=

We now have saner focusing/defocusing of the URL bar, so a lot of the custom show/hide VKB calls can go away. We show the keyboard when URL bar gets focus and not show the keyboard when it loses focus.

Comment 6

[Chris Peterson [:cpeterson]]

Comment on attachment 761170 [details] [diff] [review]
Bug 880650 - Clean up hiding/showing VKB in the awesome screen; r=

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

Nice!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c9f80387acd7 - everything with robo in the jobname was unhappy,

https://tbpl.mozilla.org/php/getParsedLog.php?id=24039251&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24039580&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24038862&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24039282&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24039492&tree=Mozilla-Inbound

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add selectAll call to URL bar initialization (v1)

Looks like I should have kept the selectAll call

Comment 10

[Chris Peterson [:cpeterson]]

Comment on attachment 762317 [details] [diff] [review]
Add selectAll call to URL bar initialization (v1)

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

oops! LGTM.

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Don't try to dismiss VKB during testBookmarksTab (v1)

With the patches in this bug, the VKB no longer appears when going back from the Edit Bookmark dialog, so we no longer need to toggle the VKB. In fact, testBookmarksTab now fails when we do toggle the VKB.

Comment 12

[Adrian Tamas (:AdrianT)]

Comment on attachment 763716 [details] [diff] [review]
Don't try to dismiss VKB during testBookmarksTab (v1)

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

The test looks good and passes locally on the Asus EEE Transformer (Android 4.0.4) and the HTC Desire Z (Android 2.3)

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5b205aacce1
https://hg.mozilla.org/integration/mozilla-inbound/rev/38f7c6ceb5d6
https://hg.mozilla.org/integration/mozilla-inbound/rev/63a1892e5298

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

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a5b205aacce1
https://hg.mozilla.org/mozilla-central/rev/38f7c6ceb5d6
https://hg.mozilla.org/mozilla-central/rev/63a1892e5298

Comment 15

[Aaron Train [:aaronmt]]

Looks like 23 is affected according to Teodora, uplift?

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Need to uplift Bug 877659 first.

Comment 17

[Aaron Train [:aaronmt]]

Mentioned that in that bug.