807030 - [Browser] "Clear History" doesn't clear Top Sites. (and there's no other obvious way to clear them)

Status: VERIFIED WORKSFORME

(Firefox OS Graveyard :: Gaia::Browser, defect, P3)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Visit some sites, in browser.

 2. Tap URL bar, and make sure some sites that you've visited are visible in "Top Sites".
     (Let's assume some of these sites are embarrassing/sensitive/private)

 3. Hit "Clear History" in the browser config page.

 4. Back out of Browser Config, and tap your URL bar again to view Top Sites.

EXPECTED RESULTS:
Top Sites should be empty.

ACTUAL RESULTS:
All my Top Sites are all still there.


NOTES: Top Sites are a function of your history, so the expectation is that they'd be cleared when your history is cleared.  As far as I can tell, it's impossible to clear your Top Sites.  ("Clear Private Data" doesn't clear them, either)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: patch

Is this good enough?

Comment 2

[Daniel Holbert [:dholbert]]

I don't know. Does it clear the actual saved top sites?  It looks like it just clears the thumbnails, but I don't really know this code.

(Also, you probably don't want to request review from me.  Someone who's changed/reviewed changes to that js file, or at least written some b2g code, would be a more appropriate reviewer. :))

Comment 3

[Daniel Holbert [:dholbert]]

We also want to be sure that cleared Top Sites don't still end up suffering from a form of bug 807026. (a flash of the old content, even after it's been cleared)  Possibly a separate issue, but one that could probably be addressed for Top Sites either here or in bug 807026.

Comment 4

[Andrea Marchesini [:baku]]

(In reply to Daniel Holbert [:dholbert] from comment #2)
> I don't know. Does it clear the actual saved top sites?  It looks like it
> just clears the thumbnails, but I don't really know this code.

It seems that the thumbnails page is not refreshed. With this patch it looks fine.
Can you suggest somebody who can review this?

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Andrea Marchesini (:baku) from comment #4)
> Can you suggest somebody who can review this?

I have no idea, but a good rule of thumb is to look at the commit history for the file (or neighboring files in its directory), to see who's reviewed (or written) code around there.

Comment 6

[Daniel Holbert [:dholbert]]

Just to clarify the situation here:
 - baku says on IRC he can't reproduce comment 0's ACTUAL RESULTS in nightly -- so this may have already been fixed (woot!)

 - However, he did notice that the thumbnails page (the browser start page) still shows a selection of your most-visited sites, so his attached patch is intended to clear that page.

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Blocking + ; looks like it's broken.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #6)
> Just to clarify the situation here:
>  - baku says on IRC he can't reproduce comment 0's ACTUAL RESULTS in nightly
> -- so this may have already been fixed (woot!)

FWIW, I can still reproduce comment 0's ACTUAL RESULTS in the release that just went out (datestamp 2012-11-01 09:44:34), so I suspect it's not fixed after all.

Comment 9

[Andrea Marchesini [:baku]]

> FWIW, I can still reproduce comment 0's ACTUAL RESULTS in the release that
> just went out (datestamp 2012-11-01 09:44:34), so I suspect it's not fixed
> after all.

Right. I cannot reproduce it on desktop because the operation is fast enough.
On device, I can still see the top list populated for few seconds and then, it disappears.

What I suggest is to have a dialog/alert/a cursor/something... and we dismiss it only when the operation is completed. Does it make sense?

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to Andrea Marchesini (:baku) from comment #9)
> Right. I cannot reproduce it on desktop because the operation is fast enough.
> On device, I can still see the top list populated for few seconds and then,
> it disappears.

Hmm, that's not what I see. ~5 minutes after I've pressed "clear history", my Top Sites pane is still populated with all the same old stuff.  (though my "history" pane is empty)

Comment 11

[Andrea Marchesini [:baku]]

it should be already fixed. So I close this bug as 'WFM'.

Comment 12

[Daniel Holbert [:dholbert]]

Verified WFM using latest stable dogfood build. (datestamp 2012-11-06 20:52:24)

Comment 13

[Daniel Holbert [:dholbert]]

I also verified that the browser start page is cleared of thumbnails for top sites, too. (so, the issue raised at the end of comment 6 is WFM as well.)

Comment 14

[Daniel Holbert [:dholbert]]

baku: you marked this as depends-on bug 807056 (I think to indicate that bug 807056's patch likely fixed this?), but I don't think that's correct.

This is already WFM in the latest stable build, released a few days ago, per comment 12, whereas bug 807056's patch only just got r+ and landed today.

Hence, removing dependency.  Perhaps you meant to mark this as depending on a different bug, and only put in bug 807056 by mistake?

Comment 15

[Andrea Marchesini [:baku]]

Right. I agree.