962612 - [User Story] Browser Chrome: View History

Status: VERIFIED FIXED

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

Description

[Ben Francis [:benfrancis]]

As a user I want to see a list of my browsing history so I can navigate to a web page I have navigated to before.

Comment 1

[Ben Francis [:benfrancis]]

Peter, I didn't see a bug filed for this. Is it part of the MVP for system browser? I'm not sure where it would live.

Comment 2

[Peter Dolanjski [:pdol]]

Thanks for filing Ben.  I was expecting the UX spec to highlight if we want to keep this functionality in the new Browser.  NI on Francis.

Comment 3

[Francis Djabri [:djabber]]

History is included in the browser start page. Latest UX spec (v0.11) on Box at: https://mozilla.box.com/s/ope5ebfg2gdxhimyhfqw

Comment 4

[Ben Francis [:benfrancis]]

As history and top sites are current browser app features, I'm guessing they're part of the system browser MVP.

Comment 6

[Kevin Grandon :kgrandon]

Eric/Francis - Do you guys know if we have visuals anywhere for the browser "newtab" page? This would include top sites and history. Thanks!

Comment 7

[Francis Djabri [:djabber]]

These should be:

Start Page:
Concept:
https://mozilla.box.com/s/qcfk0zrospvuktegalxd
Spec:
https://mozilla.box.com/s/ac65vfmuq1ywl9s2wkqc

BTW, we've collected spec locations at https://etherpad.mozilla.org/SFE-Spec-Locations for future reference

Comment 8

[Kevin Grandon :kgrandon]

(In reply to Francis Djabri [:djabber] from comment #7)
> BTW, we've collected spec locations at
> https://etherpad.mozilla.org/SFE-Spec-Locations for future reference

Thanks! To be honest I was having a hard time looking for it, and probably missed it when scrolling past. I suppose I was searching for "browser" or "new tab" though, I should've looked for "start" I suppose. Having it in this bug will help though.

Comment 9

[Eric Pang [:epang] UX]

(In reply to Francis Djabri [:djabber] from comment #7)
> These should be:
> 
> Start Page:
> Concept:
> https://mozilla.box.com/s/qcfk0zrospvuktegalxd
> Spec:
> https://mozilla.box.com/s/ac65vfmuq1ywl9s2wkqc
> 
> BTW, we've collected spec locations at
> https://etherpad.mozilla.org/SFE-Spec-Locations for future reference

that's it, thanks Francis!

Comment 10

[Kai-Chih Hu [:khu]]

Just assign an owner on user story bug. Candice, feel free to dispatch the bug to engineering managers or leads if it's much better for then to own the user story bugs. Thank you.

Comment 11

[Peter Dolanjski [:pdol]]

Dale, is this progressing?

Comment 12

[Dale Harvey (:daleharvey)]

Yup just away to get this in for review

Comment 13

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/23392

A chunk of the code here is lifted from the original browser implementation, it could be cleaner but I wanted to modify as little as possible.

The rest is modifying the datastore to keep a list of visits, it prunes at 20 and only stores one per day so we have enough information to build the view, but dont have to worry about blowing up storage.

There are 2 main cleanups, the templating of the results isnt nice, and its about time we leverage the datastore in the icon helper so we can just getIcon('http://google.co.uk') from anywhere and pick up the correct icon instead of passing around the |.icons| object everywhere, but as said was a big patch already, those should be follow ups

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8480281 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23392

I think we should probably have upgrading working after having previously visited places. What do you think?

Comment 15

[Dale Harvey (:daleharvey)]

Yeh it was working last time I tested, trying again

Comment 16

[Dale Harvey (:daleharvey)]

Comment on attachment 8480281 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23392

This issue was since we only sync the most recent items, only the most recent visits would be stored, added |asyncStorage.removeItem('latest-revision');| to trigger a resync

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8480281 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23392

Can't leave an R+ quite yet because we've broken clearing history from the settings app. Looks like we're just doing a datastore.clear() currently, and I guess this does not delete stuff from visits?

I'm making a note to add a marionette test for this if we don't have one yet.

Comment 19

[Dale Harvey (:daleharvey)]

Comment on attachment 8480281 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23392

Nice catch, yeh needed to clear the seperate visits index @ https://github.com/mozilla-b2g/gaia/pull/23392/files#diff-030b686611d2c7dd2f9f923a1ccd9699R85

Feel free to assign me the marionette test

Comment 20

[Kevin Grandon :kgrandon]

Comment on attachment 8480281 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23392

Looks good to me. Thanks!

Comment 21

[Dale Harvey (:daleharvey)]

https://github.com/mozilla-b2g/gaia/commit/f530a13b8b2e17a9cdd6e44e453122ef44ef77fc

Comment 22

[Francesco Lodolo [:flod]]

> +month-2=MATCH

You should fix this, unless you plan to start calling March with a different name. 

No need to change the ID, typo is self-explanatory in the context so localizers should be able to understand it.

Comment 23

[Kevin Grandon :kgrandon]

Follow-up landed to fix month typo: https://github.com/mozilla-b2g/gaia/commit/83cb8148699c1ffbda7ac8388a409ca48cc529e0

Comment 24

[Marcia Knous [:marcia]]

Verified fixed on Flame using:

Gaia   e7d31f0e9b6b19d9b484eeec8fb980718bc40d79
SourceStamp 532b5fb77ba1
BuildID 20140901160203
Version 34.0a1