Closed Bug 969420 Opened 6 years ago Closed 6 years ago

Rocketbar historical results are including visited web content from a hosted app, but shouldn't be

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: jsmith, Unassigned)

Details

Attachments

(2 files)

Attached image Screenshot
Build

* Version: 1.4
* Build ID: 20140206160203
* Gaia: 3fc26ae786e3869a7ef1e23afc9807ac1b4741f2
* Gecko: d05c721ea1b0

STR

1. Install wikipedia from marketplace
2. Launch wikipedia
3. Close wikipedia & open the rocketbar
4. Type w

Expected

Wikipedia's start page in the hosted app should not be present in the history results.

Actual

Wikipedia's start page in the hosted app is present in the history results.
I think this is intended behaviour, Francis?
Flags: needinfo?(fdjabri)
(In reply to Ben Francis [:benfrancis] from comment #1)
> I think this is intended behaviour, Francis?

No it is not - I already had this discussion with Francis. Content URLs visited within a hosted app aren't supposed show up in the history list - only the app itself can. Users don't need to have knowledge of the web page being visited in a hosted app - they only care about what app has been used. The URLs visited within the app don't hold meaning in an app context to a user - only the origin does.
(In reply to Jason Smith [:jsmith] from comment #2)
> (In reply to Ben Francis [:benfrancis] from comment #1)
> No it is not - I already had this discussion with Francis. Content URLs
> visited within a hosted app aren't supposed show up in the history list -
> only the app itself can. Users don't need to have knowledge of the web page
> being visited in a hosted app - they only care about what app has been used.
> The URLs visited within the app don't hold meaning in an app context to a
> user - only the origin does.

Page 33 of v0.8 of the Rocketbar spec https://mozilla.app.box.com/files/0/f/1399872384/1/f_14048520985

"Autocomplete results (history) Fast live-as-you-type local results serve up matches to visited URLs. This includes http:// URLs for installed hosted apps and http:// URLs for all other web pages. Each search result should display the page’s favicon, page title and either the URL (in the case of web pages visited in the browser) or the app name (in the case of hosted apps). Note, bookmarked URLs and app:// URLs should NOT appear within these results"

A URL is a URL, it shouldn't matter whether the app/site is installed/bookmarked or not. App URLs are slightly different because they're not real URLs on the web, which is why we don't expose them to the user or index them in the Places database.

We should de-duplicate history results with app results, but only if the URL is the same (i.e. you shouldn't see both the Wikpiedia app result and a history result for the Wikpedia homepage). I think Kevin showed a demo of this de-duplication at the work week, we probably need a bug for that filed under bug 952415 if we don't already.
(In reply to Ben Francis [:benfrancis] from comment #3)
> (In reply to Jason Smith [:jsmith] from comment #2)
> > (In reply to Ben Francis [:benfrancis] from comment #1)
> > No it is not - I already had this discussion with Francis. Content URLs
> > visited within a hosted app aren't supposed show up in the history list -
> > only the app itself can. Users don't need to have knowledge of the web page
> > being visited in a hosted app - they only care about what app has been used.
> > The URLs visited within the app don't hold meaning in an app context to a
> > user - only the origin does.
> 
> Page 33 of v0.8 of the Rocketbar spec
> https://mozilla.app.box.com/files/0/f/1399872384/1/f_14048520985
> 
> "Autocomplete results (history) Fast live-as-you-type local results serve up
> matches to visited URLs. This includes http:// URLs for installed hosted
> apps and http:// URLs for all other web pages. Each search result should
> display the page’s favicon, page title and either the URL (in the case of
> web pages visited in the browser) or the app name (in the case of hosted
> apps). Note, bookmarked URLs and app:// URLs should NOT appear within these
> results"
> 
> A URL is a URL, it shouldn't matter whether the app/site is
> installed/bookmarked or not. App URLs are slightly different because they're
> not real URLs on the web, which is why we don't expose them to the user or
> index them in the Places database.

I disagree with this. When a user installs an app & launches it, they understand local apps as a concept of the app name primarily first, not it's URL. Users typically understand two things with apps - the app name & the origin of the app that they aim to trust. If I show a user a historical result in the form of a URL, then how they are supposed know what app is involved? This is especially problematic to rely on displaying the URL in cases where the origin of the app doesn't have a direct naming scheme to the app name, which is common with a bunch of apps on marketplace. 

When I discussed this with Francis, he indicated that the app name was supposed to show up in the history, not the URL.

> 
> We should de-duplicate history results with app results, but only if the URL
> is the same (i.e. you shouldn't see both the Wikpiedia app result and a
> history result for the Wikpedia homepage). I think Kevin showed a demo of
> this de-duplication at the work week, we probably need a bug for that filed
> under bug 952415 if we don't already.

No, that's not what UX has advised to my understanding. If I installed wikipedia and I type w, I should see the wikipedia app as a suggestion, not the URL of the launch absolute path of wikipedia. It's shown in the throughout the UX spec as well - notice the results that display in large text in the history results.
Please check with Francis and have the spec updated if this is an issue. As Ben said in Comment 3, the spec clearly says,  "This includes http:// URLs for installed hosted apps". I would personally like to keep these results, but if we want to remove them we would need the spec updated first.

I'm thinking we should close this bug as the current implementation matches the spec in that regard.
(In reply to Kevin Grandon :kgrandon from comment #5)
> Please check with Francis and have the spec updated if this is an issue. As
> Ben said in Comment 3, the spec clearly says,  "This includes http:// URLs
> for installed hosted apps". I would personally like to keep these results,
> but if we want to remove them we would need the spec updated first.

There's other spots throughout the spec that clearly indicate that the app name is supposed to be used here. That was clarified in person by Francis. I filed this bug post a discussion with him on this.

> 
> I'm thinking we should close this bug as the current implementation matches
> the spec in that regard.

I think I've made it quite clear that what's proposed here doesn't make sense. Users do not understand http:// URL contexts within an app - they understand the app name.
Attached patch 969420.patchSplinter Review
Meanwhile I made patch for that issue. Can be applied if you decide to fix this bug.

Who can review the patch?
Flags: needinfo?
(In reply to Tomasz Szatkowski from comment #7)
> Created attachment 8373663 [details] [diff] [review]
> 969420.patch
> 
> Meanwhile I made patch for that issue. Can be applied if you decide to fix
> this bug.
> 
> Who can review the patch?

Let's wait until Francis gets back to us with a decision/updated spec. Once we have that we can find an appropriate reviewer. An integration test would be nice, but at a minimum we would want a unit test for this.
Flags: needinfo?
> We should de-duplicate history results with app results, but only if the URL
> is the same (i.e. you shouldn't see both the Wikpiedia app result and a
> history result for the Wikpedia homepage). I think Kevin showed a demo of
> this de-duplication at the work week, we probably need a bug for that filed
> under bug 952415 if we don't already.

I agree with this and I think this is where the confusion is coming from. If the user searches for "Wikipedia" and a match is made to the app name, then showing a history result for the wikipedia homepage is redundant. 

However, other URLs that the user visits through the app should still appear in the history results. e.g., if the user goes to the page - http://en.wikipedia.org/wiki/Russian_Revolution - through the app, and they then do a search for "Russian", the URL should appear in the history results. 

Jason has circulated an email asking Peter to clarify the requirements, so flagging Peter on need info.
Flags: needinfo?(fdjabri) → needinfo?(pdolanjski)
I think the best option this point is to continue this discussion on the email thread I've started. There's some fundamental concerns I have with the above proposal that I think needs to be addressed.
I do think that it does certainly make sense to try to launch the history item inside of the app-context if possible so any app/manifest behaviors are preserved.
Discussed offline - we're not taking the approach suggested in this bug, but we are going down a different path to handle the site vs. app use cases. Closing this out since as filed, this isn't valid anymore.
No longer blocks: rocketbar-search-mvp
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(pdolanjski)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.