STR: 1. Open a tab in browser. Navigate through a few sites; hit "back", so you've got both a "Back" and a "Forward" button available. 2. Open Browser "Settings" pane (gear icon in tab listing), and press Clear History button. Confirm when prompted. 3. Click upper-left back button to get out of Browser Settings. See if your Back/Forward buttons are still available/functional. EXPECTED RESULTS: Back/Forward buttons should be grayed out, since your browsing history should've been cleared. ACTUAL RESULTS: Back/Forward buttons are still there, and they work (they take you to pages in your history that you'd expect would have been purged). For comparison: In desktop Firefox, I get EXPECTED RESULTS after performing "Tools | Clear Recent History". So this is a place where we're breaking users' privacy expectations by diverging from desktop Firefox behavior.

It seems that purgeHistory is not called.

The Clear History button clears the global history, which is stored in IndexedDB inside the app. Session history is stored separately by the platform and as far as I know we have no API to clear session history from content.

Blocking + ; looks like it's broken.

> Session history is stored separately by the platform and as far as I know we > have no API to clear session history from content. This is what I'm going to implement. The patch is almost ready. I'm going to upload it tonight or tomorrow morning.

With this patch, purgeHistory() is added in the browserElement*.js.

Here how to use the new API in browser.js

Comment on attachment 677778 [details] [diff] [review] patch 1 I'd prefer to review this, if you don't mind, Fabrice.

Comment on attachment 677778 [details] [diff] [review] patch 1 >+ try { >+ if (history && history.count) { >+ history.PurgeHistory(history.count); >+ } >+ } catch(e) {} Why the heck does this start with a capital letter in the interface? That's bizarre. Anyway, looks great. :)

Justin, can you review and if you like, push the patch for gaia?

Comment on attachment 677781 [details] [diff] [review] Gaia patch browser 1 Although this looks fine to me, I'm not familiar with this code, so I don't feel comfortable reviewing it. The way to get a Gaia review in general is to make a pull request and ping your reviewers (in this case @benfrancis and @daleharvey). You can also flag them on bugzilla if you want to make sure there's no getting out of the review. :) People usually post an attachment which points to the github pull request.

Ok. In the meantime, we can check-in the first patch.

Because we don't have a separate bug for the Gaia work here, can we leave this open until Gaia is updated?

Sure, you can use [leave open] in the whiteboard to signify that in the future. That being said, my Bugzilla queries won't find this when I'm looking for bb+ bugs that need Aurora landing if it's still open, so adding checkin-needed so I don't forget about it.

Comment on attachment 677781 [details] [diff] [review] Gaia patch browser 1 Ben, can you review these few lines about purgeHistory() in browser.js ?

Comment on attachment 677781 [details] [diff] [review] Gaia patch browser 1 A couple of nits: 1) In the browser app we would usually use "this.tabs.foreach" instead of "for each" to which you can pass "this" as the third argument. That would mean you don't need need to do self = this. 2) We only need to to refreshButtons() for the current tab, not every tab, so it might be better to add a conditional for that purpose to prevent calling it multiple times unnecessarily.

> 1) In the browser app we would usually use "this.tabs.foreach" instead of > "for each" to which you can pass "this" as the third argument. That would > mean you don't need need to do self = this. I think this.tabs is an object. foreach is available only for arrays. is it? > 2) We only need to to refreshButtons() for the current tab, not every tab, > so it might be better to add a conditional for that purpose to prevent > calling it multiple times unnecessarily. good point.

Actually I see other places in the code where we use for each(var tab in this.tabs) ...

Ah OK, well you could do foreach on Object.keys, but if you want to leave it using for each then OK. Thanks

This patch refreshes the buttons only for the current tab. Then I saw another bug related to the iconUrl that can be null. Probably it's better to have a separated bug for this..

Andrea, this looks good thank you. Would you mind running gjslint on each of the js files: gjslint --nojsdoc filename.js and then sending a pull request on GitHub?

Done. Can we mark this bug as 'fixed' ?

Not until the PR lands, please.

https://github.com/mozilla-b2g/gaia/commit/623018214167191e6caac9672db8af5e15af74b1 patch merged.

Verified as fixed on Unagi build 20121231070201.