Closed Bug 790305 Opened 7 years ago Closed 7 years ago
Don't add history entries for redirects or error pages
I see a lot of 404's or redirects in my history. I don't want/need them there. i.e. I have a bunch of pages with the title "Problem loading page" and the neterror favicon. We currently seem to add anything we get in LocationChange. I wonder if we can check the state there first, or maybe delay adding the history entry until later?
This seems to work well. Visiting twitter.co used to add four entries (twitter.co, www.twitter.com, twitter.com, and mobile.twitter.com). With this it adds two (twitter.com which has a meta refresh and some js) which sends you to mobile.twitter.com (not sure how to filter the twitter.com address out?). Loading facebook.com goes from three redirect entries to just one entry for m.facebook.com. Invalid urls like aadvadf.adfadf or non-existant pages like dl.dropbox.com/u/72157/a don't appear. We seem to update history a lot (to much?). Details: This makes us not update on location changes or additions to the session history (but it does update the UI in these cases). It modifies things to add an entry onStateChange if the load was a success. It also changes our nsAndroidHistory.cpp implementation to not add entries if the they were a redirect or a failure. Note this has to differ from desktop where they store and filter on the transition type, allowing them to record but still ignore those visits.
Earlier today rnewman mentioned that some of his history entries have suspiciously high visit counts, perhaps caused by session restore. It looks like your change to handleSessionHistoryMessage might fix this, which would be nice.
Hmm... interesting. I'm not sure we need any of the updates in Tab.java to be honest. They seem mostly useful for updating the page titles we show in the Awesomescreen I guess. I could modify those calls so that they ONLY update, and don't create new entries.
I'm seeing typically 3 visits for each page that I visited once ever. url title date visits http://en.m.wikipedia.org Hypotonia - Wikipedia, the free encyclopedia 1348511771753 3 In fact, almost *every* page in my recent history that has never been visited on desktop looks like it has excess visits, typically 2 or 3. Here's the worst: http://www.spanishdict.co Silver in Spanish | English to Spanish Translation | Traductor español 1345736002493 84 -32 1512 I definitely haven't visited that page 84 times; once was enough! Note that it was last visited 32 days ago.
Wait, that last might be a URL collision - shared with other dictionary lookups. Still vaguely concerned by the lack of visits = 1 entries.
Comment on attachment 664606 [details] [diff] [review] Patch I need to understand more about each of these changes. I'm sure each has some affect and I'd like to know what we are really changing.
This is a bit simpler, but removes all of our adding of entries in Tabs.java, since we're already adding them in nsAndroidHistory.cpp. It also leaves the code that disables adding entries that are error pages or redirects.
Comment on attachment 674297 [details] [diff] [review] Patch v2 Any way to add a test or two here?
Attachment #674297 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/8c8c0b8e97f8 Possible to test this?
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.