Closed Bug 1150424 Opened 6 years ago Closed 6 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S11 (1may)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: huayu.li, Assigned: apastor)

References

Details

(Whiteboard: [2.2-nexus-5-l][systemsfe])

Attachments

(4 files)

Attached file 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
Attached video VIDEO0474.3gp
See Also: → 1149116
Alberto,
This bug might be a regression from bug 1149116. Could you take a look?
Blocks: 1149116
blocking-b2g: --- → 2.2?
Component: Gaia::Browser → Gaia::Search
Flags: needinfo?(apastor)
Summary: [E.me Integration]After tapping the title bar in search engine selection view,the background page will flash once. → [Search]After tapping the title bar in search engine selection view,the background page will flash once.
Assignee: nobody → apastor
Flags: needinfo?(apastor)
Could you please try if bug 1150214's patch helps? Thanks!
Flags: needinfo?(huayu.li)
Whiteboard: [2.2-nexus-5-l] → [2.2-nexus-5-l][systemsfe]
Not a critical blocker to release
blocking-b2g: 2.2? → ---
Actually, I the patch on bug 1150214 won't help. I'll take a look
Flags: needinfo?(huayu.li)
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
Flags: needinfo?(yzenevich)
(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.
Flags: needinfo?(yzenevich)
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!
Flags: needinfo?(yzenevich)
Flags: needinfo?(dale)
I wonder, if perhaps this is triggered by tapping on the top panel that overlays the value selector..
Flags: needinfo?(yzenevich)
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.
Flags: needinfo?(dale)
(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.
(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!
Attachment #8591801 - Flags: review?(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!
Attachment #8591801 - Flags: review?(kgrandon)
Hey Kevin, is this still in your radar? Thanks!
Flags: needinfo?(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 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!
Flags: needinfo?(kgrandon)
Attachment #8591801 - Flags: review?(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
Attachment #8591801 - Flags: review?(etienne) → review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S11 (1may)
Attached video 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.
QA Whiteboard: [MGSEI-Triage+]
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.
Flags: needinfo?(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.
Flags: needinfo?(kgrandon)
Attachment #8591801 - Flags: approval-gaia-v2.2?(bbajaj)
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.
Attachment #8591801 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
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
Status: RESOLVED → VERIFIED
Per comment 20 & comment 25, clear "verifyme" keyword.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.