710392 - exclude about: pages from top sites on about:home

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Comment 3

[Tim Maks van den Broek [:mad_maks]]

Ok this bug is a dup of the one i filed but what is happening with it?

-is it not consider as a bug? and we live with it
-is it a bug, who is going to work on it

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 585974 [details] [diff] [review]
patch

Review of attachment 585974 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good but shouldn't we simply avoid recording visits to about: pages in history DB altogether? For instance, I think we shouldn't show about: pages in the history tab either. Also, we don't want stock browser to have list about: pages as they're Fennec-specific. I'm giving a r- until a good reason is given to keep about: records on DB.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Desktop Firefox and XUL Fennec do not record about:* to history. However, you can bookmark an about:* page and it then will appear in the awesomebar results.

We could do the same thing in Native Fennec, but a bookmarked about:* page would likely show up in the "Top Sites" list, unless we filter. Even about:home

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 585974 [details] [diff] [review]
patch

(In reply to Lucas Rocha (:lucasr) from comment #6)
> Comment on attachment 585974 [details] [diff] [review]
> patch
> 
> Review of attachment 585974 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good but shouldn't we simply avoid recording visits to about: pages in
> history DB altogether? For instance, I think we shouldn't show about: pages
> in the history tab either. Also, we don't want stock browser to have list
> about: pages as they're Fennec-specific. I'm giving a r- until a good reason
> is given to keep about: records on DB.

1) That doesn't fix the issue for anyone that's used fennec up to now and has about:pages in their history
2) about: pages should at least show up in the Awesomebar if their bookmarked
3) given that we only show 3 or 4 top sites, I think we should exclude them from the top sites section regardess of whether they're in history or not

So, I suggest we need this whether we stop putting about pages in history or not

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 585974 [details] [diff] [review]
patch

Review of attachment 585974 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, please make sure there's a bug report about not storing about: pages in history.

Comment 10

[Steffen Wilberg]

This hides all about pages, right? Changing summary accordingly.

Comment 11

[Madhava Enros [:madhava]]

About:home is really the only about: page that should be excluded on about:home. I think about: pages should be in history, too.

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/4f11bea71a4a

Comment 13

[Aaron Train [:aaronmt]]

Samsung Nexus S (Android 4.0.3)
Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20120106 Firefox/12.0a1 Fennec/12.0a1
20120106043323
http://hg.mozilla.org/mozilla-central/rev/af6501ede378

Aurora nom?

Comment 14

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 585974 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
"top sites" section about:home can be completely occupied by about: pages, such as about:home
Testing completed (on m-c, etc.): 
on m-c
Risk to taking this patch (and alternatives if risky):
pretty risk-free

Comment 15

[Alex Keybl [:akeybl]]

Comment on attachment 585974 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/984285420b33