Closed Bug 375777 Opened 18 years ago Closed 17 years ago

javascript: urls (and other urls) that I didn't type or click on are in my history sidebar and history menu

Categories

(Firefox :: Bookmarks & History, defect)

Product:
Firefox
Component:
Bookmarks & History
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3 alpha8

People

(Reporter: moco, Assigned: moco)

References

Details

Attachments

(7 files)

screen shot
(deleted), image/png
Details
Mozilla 1.7.7 screenshot
(deleted), image/png
Details
patch
(deleted), patch
dietrich
: review+
Details | Diff | Splinter Review
history menu and sidebar before
(deleted), image/png
Details
history menu and sidebar after
(deleted), image/png
Details
url bar
(deleted), image/png
Details
from ispiked's places.sqlite, compare sidebar (no javascript urls) to url bar (again, fix coming in bug #389491)
(deleted), image/png
Details
javascript urls are in my urlbar autocomplete and in my history sidebar! screen shot coming...
Attached image screen shot (deleted) —
Attached image Mozilla 1.7.7 screenshot (deleted) —
Yeah, and? Well, I have to admit I had that exact same "uh oh, that's not right" reaction myself a week or two ago, but, this is a screenshot of the 1.7.7 suite, oldest thing I had laying around, doing the exact same thing, and the code makes it seems to be what's been intended all along: javascript: URLs that have output are supposed to wind up in history. So, why were we both surprised? Did Fx break it for a while, and only fairly recently (2.0.0.3 does the exact same thing as trunk and my ancient suite) fix it?
I use this all the time. I don't think there's anything wrong with it, as long as only previously *typed* javascript: URLs are in the autocomplete list.
Summary: javascript urls are in my urlbar autocomplete and in my history sidebar! → javascript: urls are in my urlbar autocomplete and in my history sidebar!
Could this have been a side effect of switching to places-based global history? IIRC in Firefox 2 these URLs are added to session history but not global history, so they persist for the session but not after a restart (see e.g. bug 359115).
fwiw, I'm seeing javascript:"" in my history sidebar, even though I've never typed it. phils comment about "javascript: URLs that have output are supposed to wind up in history" is interesting. (for example, javascript:alert("boo"); does not show up in my history. as for what jesse wrote: "I don't think there's anything wrong with it, as long as only previously *typed* javascript: URLs are in the autocomplete list" I think that makes sense. currently, we exclude embedded visits from the history menu, autocomplete and sidebar, but I'm not sure why javascript:"" is ending up in there, unless that is some sort of dummy url that I've clicked on? I think we should figure this out before fx 3 ships.
Flags: blocking-firefox3?
OS: Windows XP → All
Hardware: PC → All
Summary: javascript: urls are in my urlbar autocomplete and in my history sidebar! → javascript: urls that I didn't type are in my urlbar autocomplete and in my history sidebar
in my places.sqlite file, these visits have type 0, which isn't supposed to happen. (1 - 6 are valid, see http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/public/nsINavHistoryService.idl#1154) so I've added code to throw me into the debugger if we attempt to call nsNavHistory::InternalAddVisit() with 0 as the transition type. but going forward, all the places in places SQL where we do: visit_type <> 4 we should do visit_type <> 4 and visit_type <> 0
Assignee: nobody → sspitzer
> I've added code to throw me into the debugger The problem comes from nsNavHistory::AddVisitChain(), because we don't have a referrer. we don't have a referrer when: 1) opening a new window, and we load the hope page 2) on restart, we reload tabs (and in those pages, we have embedded vists, for example, http://mail.google.com/mail/html/load.html for gmail.) our current code does: if (CheckIsRecentEvent(&mRecentTyped, spec)) transitionType = nsINavHistoryService::TRANSITION_TYPED; else if (CheckIsRecentEvent(&mRecentBookmark, spec)) transitionType = nsINavHistoryService::TRANSITION_BOOKMARK; but if it's not a recently typed url or a recently clicked bookmark, transition type is left as 0. On thing to do if (CheckIsRecentEvent(&mRecentTyped, spec)) transitionType = nsINavHistoryService::TRANSITION_TYPED; else if (CheckIsRecentEvent(&mRecentBookmark, spec)) transitionType = nsINavHistoryService::TRANSITION_BOOKMARK; else if (aToplevel) transitionType = nsINavHistoryService::TRANSITION_LINK; else transitionType = nsINavHistoryService::TRANSITION_EMBED; That would keep "embedded visits" (like to http://mail.google.com/mail/html/load.html for gmail) out of history. the same would go for javascript:"" (but I need to figure out a test case for that.) but would add visits for when opening a new window (when you set prefs to load your home page) Other examples (similar to javascript:"") from ispiked's places.sqlite file: javascript:false; javascript:"; javascript:'<html></html>' javascript:'frame1' javascript:'frame2' additionally, I still want to fix our SQL so that we ignore visit_type <> 0 which has creeped into places.sqlite so we don't show those links in the history sidebar, history menu, url bar autocomplete, etc.
Status: NEW → ASSIGNED
Summary: javascript: urls that I didn't type are in my urlbar autocomplete and in my history sidebar → javascript: urls (and other urls) that I didn't type or click on are in my urlbar autocomplete and in my history sidebar
Target Milestone: --- → Firefox 3 M8
That sounds like the right approach.
Flags: blocking-firefox3? → blocking-firefox3+
Attached patch patch (deleted) — — Splinter Review
Attachment #274637 - Flags: review?(dietrich)
Attached image url bar (deleted) —
note, for the url bar, this is not fixed. our currently URL bar query does not join against moz_historyvisits (http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp#272) The fix for the url bar autocomplete part of this bug is contained in a patch for bug #389491
steps to reproduce the problem, at least with gmail: do this in a build without the fix: 1) create new profile (so our prefs are set to load homepage on startup) 2) set http://mail.google.com/mail/ as your home page 3) log into gmail, choose remember me 4) load about:blank 5) clear history 6) exit 7) restart you should get the load.html in your history sidebar (https://bugzilla.mozilla.org/attachment.cgi?id=274638) because we were not setting that non-top level load as TRANSITION_EMBED. start up a build with the fix, and you won't see those old loads (as I added visit_type <> 0) and we won't add new loads to your places.sqlite without TRANSITION_EMBED (you'll need SQLite Database Browser to verify that part.)
Comment on attachment 274637 [details] [diff] [review] patch looks ok, r=me
Attachment #274637 - Flags: review?(dietrich) → review+
morphing to reflect what this fixes. I'll log a new bug about the url bar, which will be fixed by the patch in bug #389491 for a places.sqlite to test this bug against, see bug #384677
Summary: javascript: urls (and other urls) that I didn't type or click on are in my urlbar autocomplete and in my history sidebar → javascript: urls (and other urls) that I didn't type or click on are in my history sidebar and history menu
fixed. Checking in nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHis tory.cpp new revision: 1.150; previous revision: 1.149 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I used the steps in comment 15 with the build from 6/10 (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a6pre) Gecko/2007061004 Minefield/3.0a6pre). I can't reproduce this issue. Is there a specific regression range for this? All I get in my history menu or sidebar is the inbox page.
No longer blocks: 392392
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: