1150424 - [Search]After tapping the title bar in search engine selection view,the background page will flash once.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Search, defect)

Description

[Alissa（leave from Mozilla）]

Attachment: logcat1.txt

[1.Description]:
This issue is submitted by comment 13 of Bug 1149116.
[Nexus5 v2.2&v3.0][Flame v2.2&v3.0][E.me Integration]In the search engine selection view, try to tap the the title bar, the background page will flash once,.
Found time:16:27
see attachments:VIDEO0474.3gp,logcat1.txt

[2.Testing Steps]: 
1.Launch Search
2.Input some keywords
3.Tap search engine
4.Tap  the title bar in search engine view


[3.Expected Result]: 
4.It should not have any response while tapping title bar.

[4.Actual Result]: 
4.The background page will flash once.

[5.Reproduction build]: 
Flame v2.2:[Affected]
Build ID               20150401002624
Gaia Revision          8b3086ad3963f1707e2bee9094baccafffe161c4
Gaia Date              2015-03-31 21:48:06
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/20b67213a047
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150401.042225
Firmware Date          Wed Apr  1 04:22:36 EDT 2015
Bootloader             L1TC000118D0

Flame 3.0[Affected]
Build ID               20150401160204
Gaia Revision          4bb3a933bd805e8df1e11827cb247754c3565b0b
Gaia Date              2015-04-01 02:06:11
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e044f4d172e2
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150401.193001
Firmware Date          Wed Apr  1 19:30:12 EDT 2015
Bootloader             L1TC000118D0

Nexus5 2.2[Affected]
Build ID               20150401002624
Gaia Revision          8b3086ad3963f1707e2bee9094baccafffe161c4
Gaia Date              2015-03-31 21:48:06
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/20b67213a047
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150401.041913
Firmware Date          Wed Apr  1 04:19:28 EDT 2015
Bootloader             HHZ12d

Nexus5 3.0[Affected]
Build ID               20150401160204
Gaia Revision          4bb3a933bd805e8df1e11827cb247754c3565b0b
Gaia Date              2015-04-01 02:06:11
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e044f4d172e2
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150401.192533
Firmware Date          Wed Apr  1 19:25:49 EDT 2015
Bootloader             HHZ12d

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[Alissa（leave from Mozilla）]

Attachment: VIDEO0474.3gp

Comment 2

[Hermes Cheng (inactive after July 27, 2015)]

Alberto,
This bug might be a regression from bug 1149116. Could you take a look?

Comment 3

[Alberto Pastor [:albertopq]]

Could you please try if bug 1150214's patch helps? Thanks!

Comment 4

[Candice Serran (:cserran)]

Not a critical blocker to release

Comment 5

[Alberto Pastor [:albertopq]]

Actually, I the patch on bug 1150214 won't help. I'll take a look

Comment 6

[Alberto Pastor [:albertopq]]

I'm not too familiar with this code, but the problem is that we are receiving a '_inputmethod-contextchange' with 'blur' inputType when clicking on the title. When that happens, we are hiding and then showing again the value selector.

:yzen, any idea?

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/value_selector/value_selector.js#L182

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Alberto Pastor [:albertopq] from comment #6)

Hmm, for some reason I can't seem to reproduce this issue. I m typing the search text, select provider but can't get the value selector flickering. 

> I'm not too familiar with this code, but the problem is that we are
> receiving a '_inputmethod-contextchange' with 'blur' inputType when clicking
> on the title. 

Which title do you mean exactly here? The one that belongs to the value selector?

> When that happens, we are hiding and then showing again the
> value selector.
> 
> :yzen, any idea?
> 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> value_selector/value_selector.js#L182

If indeed it is caused by the above line, I would I would see in addition to checking that it's 'blur' I would also check that the target is not inside the value selector just in case and only then hide.

Comment 8

[Alberto Pastor [:albertopq]]

Hi! 

Yes, I mean when clicking anywhere in the row where it says 'Select'. What build are you using? Should be easily repro on latest master.
Regarding your suggestion, the problem is that we always receive the events from the same target, the one we are listening events from (div id='search', in this case).

Dale, do you have any other ideas here?

Thanks!

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

I wonder, if perhaps this is triggered by tapping on the top panel that overlays the value selector..

Comment 10

[Alberto Pastor [:albertopq]]

That seems to be the problem... Changing the z-index of the #top-panel fixes it.

Thanks! I'll try to figure out how to solve it.

Comment 11

[Kevin Grandon :kgrandon]

(In reply to Alberto Pastor [:albertopq] from comment #10)
> That seems to be the problem... Changing the z-index of the #top-panel fixes
> it.

Is that going to be the fix? If we change the z-index then we prevent the utility tray from sliding down, don't we? Probably not the end of the world, but kind of nice if we still have access to it.

Comment 12

[Alberto Pastor [:albertopq]]

(In reply to Kevin Grandon :kgrandon from comment #11)
> (In reply to Alberto Pastor [:albertopq] from comment #10)
> > That seems to be the problem... Changing the z-index of the #top-panel fixes
> > it.
> 
> Is that going to be the fix? If we change the z-index then we prevent the
> utility tray from sliding down, don't we? Probably not the end of the world,
> but kind of nice if we still have access to it.

I didn't mean that was the fix, just that helped to identify the problem. I'm trying to figure out what options do we have. Thanks!

Comment 13

[GitHub Autolander Bot]

Attachment: [gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8591801 [details] [review]
[gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

Left a question on github. Could you address and re-flag me? Thanks!

Comment 15

[Alberto Pastor [:albertopq]]

Hey Kevin, is this still in your radar? Thanks!

Comment 16

[Kevin Grandon :kgrandon]

(In reply to Alberto Pastor [:albertopq] from comment #15)
> Hey Kevin, is this still in your radar? Thanks!

Oops, it was not - but leaving the needinfo flag on me so it is.

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 8591801 [details] [review]
[gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

I'd like another set of eyes to take a look at this if possible. Seems like this should fix it, but I'm still not sure why you need the mouse events. It seems like calling stopPropagation() on the touch events should also cancel the mouse events? Etienne - would you mind taking a look at this? I'm good if you're happy with it. Thanks!

Comment 18

[Etienne Segonzac (:etienne)]

Comment on attachment 8591801 [details] [review]
[gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

* preventing default on mousedown should be enough if it's a focus issue (not a blocker if we need to prevent on other events but we should double check that it's really needed)
* we should remove the topPanel from the keyboardimeswitchershow/hide handling if we already preventDefault on it
* some tests nits on github

r=me with that, let me know if anything's unclear

Comment 19

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/48e54f27db0bf2195940dd7f96399102a05d2338

Comment 20

[Shine(Leave from Mozilla)]

Attachment: Verify_video.mp4

This problem is verified pass on latest build of Nexus5 3.0 and Flame 3.0 by the STR in comment 0.
See attachment: Verify_video.mp4
Rate: 0/10
Device: Flame 3.0 (Pass)
Build ID               20150427160201
Gaia Revision          0636405f0844bf32451a375b2d61a2b16fe33348
Gaia Date              2015-04-27 16:42:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/caf25344f73e
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150427.192938
Firmware Date          Mon Apr 27 19:29:49 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 3.0 (Pass)
Build ID               20150427160201
Gaia Revision          0636405f0844bf32451a375b2d61a2b16fe33348
Gaia Date              2015-04-27 16:42:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/caf25344f73e
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150427.192534
Firmware Date          Mon Apr 27 19:25:51 EDT 2015
Bootloader             HHZ12f

Leaving verifyme for 2.2 uplift/verification.

Comment 21

[Hermes Cheng (inactive after July 27, 2015)]

Kevin,
I was wondering if we can also uplift the patch to 2.2 although it is not a blocker. 
The flashing experience in 2.2 is not good.

Comment 22

[Kevin Grandon :kgrandon]

Comment on attachment 8591801 [details] [review]
[gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

Requesting uplift for comment 21.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Possibly a regression.
[User impact] if declined: Poor ux using rocketbar.
[Testing completed]: Manual and unit testing.
[Risk to taking this patch] (and alternatives if risky): Fairly low risk, smallish patch.
[String changes made]: None.

Comment 23

[bhavana bajaj [:bajaj]]

Comment on attachment 8591801 [details] [review]
[gaia] albertopq:1150424-tray-provider-select > mozilla-b2g:master

Only approving this as this is possible a regression and the patch is low risk. Adding verifyme for qa verificaiton.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/b4f249193814b4e22d0aab2d4b8428bc2317a54a

Comment 25

[Hermes Cheng (inactive after July 27, 2015)]

verify 2.2 with below test env

Build ID               20150503002500
Gaia Revision          8d14361337e608c8cdf165ea5034db5eda23b618
Gaia Date              2015-05-01 18:23:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150503.040203
Firmware Date          Sun May  3 04:02:15 EDT 2015
Bootloader             L1TC000118D0

Comment 26

[Shally Li（leave from Mozilla）]

Per comment 20 & comment 25, clear "verifyme" keyword.