Closed
Bug 1054234
Opened 10 years ago
Closed 10 years ago
[Flatfish][Rocket bar] In search page, typing 'p' text will appear Phone app.
Categories
(Firefox OS Graveyard :: Gaia::Search, defect)
Firefox OS Graveyard
Gaia::Search
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: eva.chen.fx, Assigned: chens)
References
Details
(Whiteboard: [Flatfish][TCP])
Attachments
(4 files)
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36 Steps to reproduce: 1. Update a Flatfish to BuildID: 20140815012352 2. Open Settings app. 3. Tap the Rocket bar. (Rocket bar: The left side of the Rocket bar displays the title of the current app.) 4. Typing 'p' text into the 'Search or enter address' fields. ----------------------------------------------- Environment Info: BuildID: 20140815012352 Gaia: e57ea93196e997336934d6e47323755c5bdd80e9 Gecko:1e2086d3a6f473b2437f25a4a9226b6c2d6c2605 B2G: 2.1.0.0-prerelease Platform version : 34.0a1 Repro frequency: 100% See attached: Bug_Phone App.jpg Actual results: <ISSUE> In search page, typing 'p' text will appear Phone app. The Flatfish doesn't support telephony, so that is incorrect.
Assignee | ||
Comment 1•10 years ago
|
||
Github pull request: https://github.com/mozilla-b2g/gaia/pull/22969
Assignee: nobody → shchen
Attachment #8474354 -
Flags: feedback?(kgrandon)
Assignee | ||
Comment 2•10 years ago
|
||
Hi Kevin, I found in the original homescreen we will check each app's launch path to determine to process this app or not, do we have to handle this in the vertical one? [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L1540-L1549 [2] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L1162
Flags: needinfo?(kgrandon)
Comment 3•10 years ago
|
||
Comment on attachment 8474354 [details] [review] WIP Wow, this setting is a giant hack. We really need to get rid of entry_points so we can get rid of it. If you're going to do it, don't do it in every search. Instead do it outside of the autocomplete flow: https://github.com/shamenchens/gaia/blob/Bug1054234-SearchBlacklistApp/apps/search/js/providers/local_apps.js#L21
Attachment #8474354 -
Flags: feedback?(kgrandon)
Comment 4•10 years ago
|
||
(In reply to Sherman Chen [:chens] from comment #2) > Hi Kevin, > > I found in the original homescreen we will check each app's launch path to > determine to process this app or not, do we have to handle this in the > vertical one? Perhaps you could add this logic here: https://github.com/mozilla-b2g/gaia/blob/master/apps/verticalhome/js/sources/application.js#L199
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #3) > If you're going to do it, don't do it in every search. Instead do it outside > of the autocomplete flow: > https://github.com/shamenchens/gaia/blob/Bug1054234-SearchBlacklistApp/apps/ > search/js/providers/local_apps.js#L21 I found it's not sufficient to do it outside the autocomplete flow, within the find function it will still looking for entry_points for each app: https://github.com/shamenchens/gaia/blob/Bug1054234-SearchBlacklistApp/apps/search/js/providers/local_apps.js#L94-L99 Looks like it also handle inside find function or to update the key to store the app, I will provide another WIP patch later.
Flags: needinfo?(kgrandon)
Comment 6•10 years ago
|
||
Ok, my recommendation is that we should build appListing externally to the find() function. Are we in any kind of rush here? I assume that 2.1 on tablet is only for developers at this point?
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 7•10 years ago
|
||
Ah, I got your point, will try to separate appListing later. And you are right, currently 2.1 on tablet is for developer only so it's no rush here. I'm just curious about what things can be done on tablet :)
Assignee | ||
Comment 8•10 years ago
|
||
Hi Kevin, WIP updated to separate appListing and add unit test. Could you take a look and give some feeback? thanks. Github pull request: https://github.com/mozilla-b2g/gaia/pull/23030
Attachment #8475041 -
Flags: feedback?(kgrandon)
Comment 9•10 years ago
|
||
Comment on attachment 8475041 [details] [review] WIP 2 Not happy about having to hit settings again, but this approach should be fine. Thanks.
Attachment #8475041 -
Flags: feedback?(kgrandon) → feedback+
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8475041 [details] [review] WIP 2 Pull request updated, could you review this one?
Attachment #8475041 -
Flags: review?(kgrandon)
Comment 11•10 years ago
|
||
Comment on attachment 8475041 [details] [review] WIP 2 Left a few comments on github. Let me know if you have any questions.
Attachment #8475041 -
Flags: review?(kgrandon)
Assignee | ||
Comment 12•10 years ago
|
||
Hi Kevin, Patch updated and comments are addressed, could you take a look and give feedback? thanks. Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23130
Attachment #8476473 -
Flags: feedback?(kgrandon)
Comment 13•10 years ago
|
||
Comment on attachment 8476473 [details] [review] WIP 3 Ok, this is much better, and should be quite performant - thanks! I didn't see anything wrong with this and it seemed to work, so I'm fine leaving an R+ here. I do want to be clear that this is a terrible hack and needs to go away. I've filed bug 1056874 so we can track this.
Attachment #8476473 -
Flags: feedback?(kgrandon) → review+
Assignee | ||
Comment 14•10 years ago
|
||
Merged to master: https://github.com/mozilla-b2g/gaia/commit/02e61f6054717699f773448907707f5246f863f6
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 15•10 years ago
|
||
Verified okay and close the bug. Enviromental Variables: ---------------------------------------------------- Device : Flatfish 2.1 Master B2G : 2.1.0.0-prerelease Gaia : b5aed82b1d8750a53848479b6400902d920d1475 Merge: 553f796 7a06c70 Author: Cristian Rodriguez <crdlc@tid.es> Date: Thu Aug 28 08:21:36 2014 +0200 Gecko : 64c4bef1c1234d2fdd60974ed30d1034ec570c34 BuildID : 20140821013807 Version : 34.0a1
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•