From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.17pre3 i686; en-US; m16) Gecko/20000619 BuildID: 2000061920 The autocompletion feature does not autocomplete urls which were reached by following links, as it is done in NS4.x and IE>=4. Reproducible: Always Steps to Reproduce: 1.click on a link 2.try typing in the link manually Actual Results: no autocompletion is done Expected Results: as in NS4, those URLs should be autocompleted too

Taking. No clue when I will be able to get to it. Not sure if we want to keep this Nav 4.x behavior in mozilla. cc'ing johng for comments

If it is an easy fix that you might get to anyway when fixing bugs in autocomplete, and you feel strongly about it, then go ahead and nominate nsbeta3. Otherwise, ignore it and leave the milestone as "Future" - I'm comfortable shipping without this feature.

re-summarising

nav triage team: looked at this bug, it is not a beta stopper. bulk update of several such bugs.

moving these bugs to History: URLBar

I think the summary should be changed. It seems like he's requesting that there be no autocompletion for clicked links. I find that this functionality would be useful. Also, would a way to accomplish this feature be to compare the typed text to the urls in history?

changing summary so that it reflects the wanted feature...

Added "(link url bar autocomplete to global history)" to summary which I think correctly reflects what bug is about. Alec mentioned looking into this when we talked yesterday, well, congrats, it's on your 0.9 plate. ;-) Marking nsbeta1+, mozilla0.9

before I do any urlbar work, I wanted to clean this file up a fair amount... looking for reviews.

res = nsServiceManager::GetService(kRDFServiceCID, NS_GET_IID(nsIRDFService), - (nsISupports **)&gRDFService); + (nsISupports **)&gRDFService); res = nsServiceManager::GetService(kRDFCUtilsCID, NS_GET_IID(nsIRDFContainerUtils), - (nsISupports **)&gRDFCUtils); + (nsISupports **)&gRDFCUtils); It looks like |gRDFService| is only used in the constructor, so no need to have it as a global static. |gRDFCUtils| is used in a few places, but why have it as a static instead of as a member variable and make it a nsCOMPtr (and use do_GetService)? + nsCOMPtr<nsIPref> prefs = do_GetService(kPrefServiceCID, &res); Shouldn't that be NS_PREF_CONTRACTID so you can get rid of the CID? Same for |NS_RDF_CONTRACTID "/rdf-service;1"| and |NS_RDF_CONTRACTID "/container-utils;1"| (ugh, is there a bug on converting those to #defines?)... +nsUrlbarHistory::SearchCache(nsAReadableString& searchStr, and +nsUrlbarHistory::GetHostIndex(nsAReadableString& aPath, PRInt32 * aReturn) add a |const|. Actually, when I tried compiling with the patch, gcc complained about this in a rather odd way: nsUrlbarHistory.cpp: In method `nsresult nsUrlbarHistory::OnStartLookup(const PRUnichar *, nsIAutoCompleteResults *, nsIAutoCompleteListener *)': nsUrlbarHistory.cpp:240: no matching function for call to `nsUrlbarHistory::SearchCache (nsLiteralString, nsCOMPtr<nsIAutoCompleteResults> &)' nsUrlbarHistory.h:56: candidates are: nsresult nsUrlbarHistory::SearchCache(nsAReadableString &, nsIAutoCompleteResults *) <near match> + if ((const char*)preHost) I don't think the cast is needed (gcc doesn't need it, anyway). - if (!aItem || !aArray) + if (!aArray) Why not check for aItem.IsEmpty() ? + printf("nsUrlbarHistory::OnStopLookup()\n"); and +#if 1 + printf("nsUrlbarHistory::OnAutoComplete(%s)\n", NS_ConvertUCS2toUTF8(searchString)); +#endif shouldn't those be wrapped in #ifdef DEBUG or DEBUG_alecf ? Oh, and that NS_ConvertUCS2toUTF8 needs a .get(). + if (nsCRT::strcmp(aTopic, NS_LITERAL_STRING("nsPref:changed").get())==0) { How about: if (nsLiteralString(aTopic).Equals(NS_LITERAL_STRING("nsPref:changed"))) { or if (NS_LITERAL_STRING("nsPref:changed").Equals(aTopic)) {

Hey Alec, have you done any work on this yet? If not, I can spare you. I've got this working now in my tree, but I need to polish it a bit. Need to be smart about the way hostnames are treated, and a bunch of other little details. I'll have a patch by monday, hopefully.

adding hewitt so I can answer his question (shame on you for asking a question without adding yourself to the CC :)) nope, I haven't - I'm really whacking nsUrlbarHistory.cpp though, so if you're touching that I'd prefer you wait until I land the above patch.

As far as I can tell, all of the autocomplete-related stuff in nsURLBarHistory is useless at this point. If we're going to hook autocomplete up to the global history, that will all be done in nsGlobalHistory, and at that point the only thing you'll need nsURLBarHistory for is to add/remove items in the datasource.

well, my eventual goal with this is to have the url bar store everything in global history, by flagging certain urls as 'typed' (as in a url that the user has typed into the browser) - then when we do autocomplete, we first match against the 'typed' urls, and then agains the entire history.

I've reached a point in developing the new autocomplete widget where I'm happy enough to check in what I have now, but think in the long run there are many problems yet to resolve. I have a forthcoming patch which is ready to be reviewed (gently). There are still a few outstanding issues with autocomplete that this patch does not address, namely: 1. don't support multiple datasources yet 2. don't support incremental rebuilding of the list 3. need a speedy way to deal with large result sets I've spoken to dmose a bit about these issues, as he has certain needs for LDAP autocomplete. It would be nice if we could be smarter about the way we build the list. Currently, we kick off an asynchronous search each time the user enters a new character, and that search must callback just once with the entire list of results. Then, all menuitems are removed and new ones are added to the list. This can be slow for large lists, and currently I have a bit of a hack in place which makes it faster in some situations (see "rangeOptimize" in the patch). What I would really love to be able to do is use the outliner widget to solve these problems. Because the slowest part of autocomplete is not the actual search, but the building of the content model, it would be swell if we could use outliner within a popup to solve this. I ran a test and found out that outline doesn't like popups -- it crashes when you try to open a popup which contains an outliner. On the bright side, if you apply this patch you'll see autocomplete kicks so much more ass than it did before, so it's a worthy improvement even if it's not perfect. I think we could get away with pushing the above issues off to a future milestone. As if I haven't written enough already, here's a list of the things that the following patch accomplishes: -- popup spans the entire textfield -- popup stays open while you type, and updates immediately -- a scrollbar appears if the results list is too long -- up/down/pageup/pagedown navigation works for scrollable lists -- hitting escape while keying through results reverts to previous typing -- option to fill and select text as user types (used in msgcompose, but not urlbar) -- ability to filter user-entered text before passing it to the autocomplete session -- built in support for (optional) proxy icon -- built in support for (optional) "history" menubutton (which spans entire textfield) -- the textbox context menu is shared via an XBL binding (should be done for all textbox bindings too) -- widget used in Navigator urlbar, Open Location Dialog (new), msg compose, and abook mailing list dialog -- in Navigator, hooked up urlbar autocomplete to the global history -- in Navigator, the autocomplete results list is sorted alphabetically -- in Navigator, search engine option appears at the bottom of the popup -- in Navigator, clicking on a url in the list automatically loads it -- in Navigator, search is smart about chopping off "http://" and "www." before matching, e.g. typing "cnn" will match "http://www.cnn.com/news/blah.html" -- in Navigator, the proxy icon disappears while the user is typing

wow, that is pretty kickass but one question that came up in another bug somewhere: "-- in Navigator, clicking on a url in the list automatically loads it" suppose I've already been to http://www.foobar.com and now i would like to get to http:// www.foobar.com/somepage.html? Imagine foobar is some long annoying URL. When i start typing foo... i want it to complete but then i want to add to it. How do i do that? with the mouse? with the keyboard? Just asking

taking this one

Had a look at this. Things look good over all. Need to make sure you add a patch for the jar.nm It was missing for all of your new files. Also was not able to get the URL bar to show up in the classic theme. I think it was my build but can you verify that it works in your build. Other then that it looks good.

I've got one more patch coming up with includes the following enhancements: 1. the patch should actually apply! (thanks alec) 2. support for multiple search sessions 3. no more stupid autocomplete menubutton 4. support for mac classic theme These are the caveats I'm aware of: 1. autocomplete won't work in navigator until you load a page (seems the history isn't loaded until then) 2. that's it! I'd appreciate if lots of people could apply this patch and give it a test drive. Since the urlbar is such a sensitive area of the product, I'd hate to bust it for a day. Try to get over the fact that the patch is 3000+ lines long and 34 files wide. Thanks!

I would like to add that testing message compose is also very important :-)

I spent my entire day today stress-testing the autocomplete widget and I found a few cracks in it's armor. I focused particularly on making the message compose autocomplete work smoothly. Little annoying things would happen if you typed too fast, tabbed or clicked out of the widget, etc... I think I've plugged all those holes now, though. Additionally, in Navigator I wrote a little more code to ensure that the page-proxy-button is invisible and not draggable when the user is typing a url. It returns to it's normal state when the value of the urlbar becomes a valid page which had been loaded successfully. I hate to keep posting patch after patch after patch to this bug... so I'll hold off on posting my new patch until sunday or monday in case I tweak it more over the weekend. If you want the latest patch sooner than that, ping me.

Another late-breaking feature -- I just enabled navigator autocomplete to display the title of the page in addition the the url of each autocomplete result.

Wow, hewitt is laying the smackdown on this bug today. Question: is this just Linux? If not, should we change the OS to "All"?

Have you tested it agains Japanese input? That was a concern when I wrote the original autocomplete widget. You can easely test that on Mac. Mac OS 9 CD have all you need to install Japanese input support.

Nice work! The multiple sessions thing works! Here are some random comments about the existing patch: * the sessions' OnStopLookup()s never seem to get called at all. This causes problems with LDAP, because it expects (reasonably, I think) that any running search will be stopped before a new one is begun. * For the mail compose autocomplete, we'd like to be able to complete against multiple addressbooks of a single type at a time This has a couple of implications: * Each entry in mSessions should probably be an array. * The autocomplete session of a particular type may no longer be a service. In the case of LDAP, there would be one object implementing nsIAutoCompleteSession and nsILDAPAutoCompleteSession per directory. In order to set the LDAP-specific params, I need to QI each session to nsILDAPAutoCompleteSession. Since I won't be able to get the session just with a getService() from JS like the current compose-window addrbook completion code does, mSessions would need to be a public property (like autoCompleteSession was in the old code) so that I can iterate over the different LDAP sessions. I would be really interested in continuing to see your up-to-the-minute patches, as I'll be hacking on this over the weekend. Seems to me like it would be completely reasonable to keep attaching updated patches here; but email them to me if you'd rather. Anyway, this is great work! I'm amazed and pleased that I was actually able to get a composite addrbook/ldap session working right out of the box. Thanks!

Here comes my latest patch. I've just tested on mac and linux today and everything looks real good. Well... almost. One particular problem I found is that when I retrieve the page title from the history mork db, on mac only I'm seeing garbage at the end of the string, like "mozilla.org0??$??". I don't know too much about mork, but perhaps there's a different in the way strings are stored on mac vs. windows?

I hate to move this to 0.9.1, but I think it's for the better. I'm going to take one last stab at improving this widget by re-writing the drop-down list using outliner. This change is absolutely necessary because I have found that the time spent building the set of menuitems is significant on old systems, and even debug builds on new systems.

is there a build with this patch available to the public? I'd really like to try this out but I don't have the time to cvs co/patch/build/etc. any chance that a build can be put in ftp.mozilla.org/pub/mozilla/nightly/experimental/?

Right now I'm working on re-writing the widget so that it uses outliner. It's just way too slow building all those menuitems. It might take me a few days before I have a new patch, depending on how many outliner bugs I turn up (2 already)...

Just an update -- outliner is cooperating nicely, and I've got the widget about 90% re-written. There are two dependent bugs at this point, but I've got fixes for both in my tree which I'll try to land soon. The good news is that the performance boost I expected to get by switching to outliner has definitely happened. No more ui-freezes when you get a really long result list.

Great! Even a partially working patch would be very handy at this point, because the CVS tip has now diverged enough that the most recent patch here no longer works. :-(

Working on the outliner-conversion this week has been far less pleasant than I had expected. I have been continually tripping over layout bugs, and am wasting a lot of time fixing these bugs that get in my way (see the 4 dependent bugs). Nevertheless, I think I'm almost done! The thing works beautifully and I've re-implemented about 95% of the functionality of the menuitem-based widget at this point. I'll post a "not done yet" patch shortly for the benefit of dmose and others who might care. In addition to this patch, you'll need to apply patches from the 4 dependent bugs above.

The patch I'm about to submit actually works. Really well. Considering what a disaster things were yesterday, I'm surprised. Long live outliner! So, go to town all you eager reviewers! :) Remember, you have to apply the patches from the 4 blocking bugs above (76297,76428,76465,76894).

comments on the global history part of the patch: * you're leaking in nsGlobalHistory::AutoCompleteEnumerator::IsResult - I'm not sure why you're doing this: nsLocalString(url.ToNewUnicode()); you should just be able to pass in "url" - if you're running into zero-termination issue, find me on AIM/IRC and we'll figure it out. * In ConvertToISupports() you shouldn't need to use ToNewUnicode() here either, you should just be able to say .get() * when you're converting the enumerator to the nsISupportsArray, you have: + while (NS_SUCCEEDED(enumerator->GetNext(getter_AddRefs(entry)))) { + nsCOMPtr<nsIAutoCompleteItem> item = do_QueryInterface(entry, &rv); + if (NS_SUCCEEDED(rv)) + array->AppendElement(item); + } I think you should assume, since you own all the code that creates this enumerator, that each element is an nsIAutoCompleteItem - so I wouldn't bother with the extra QueryInterface, just dump it into the array. Also, I'm pretty sure you should be using HasMoreElements, just to be consistent with the API. - I'm not sure I'm keen on the idea of converting the enumerator -> nsISupportsArray -> flat array -> sort -> nsISupportsArray.. at the very least, we're doing a LOT of ADDREF/RELEASEes to do this conversion. Might I suggest you use nsAutoVoidArray instead of the first nsISupportsArray? - this: + delete items; should be + delete[] items; I'm not sure I'm so happy about the http-specific logic in here, but I'm not sure of a better solution at the moment. I'd like to see a protocol-independant way of deciding if two urls are a "close match" It also seems like we really need to have a getValueConst() in nsIAutoCompleteItem, so that we don't have to keep allocating copies of the data in there just to compare it to another string. that's it for now, I'll revisit this on monday.

All the patches apply nicely, but upon trying to build, autocomplete.js appears to be missing. :-/

I've got one more Happy-Fun-Patch for all you kids to enjoy. But remember, kids, do not taunt Happy-Fun-Patch. Do not look directly at Happy-Fun-Patch. And do not attempt to ignite Happy-Fun-Patch. It's way too late (or, early?) and I'm tired. Note to Alec: I addressed several of your comments in the this revision. When you next attempt to apply/review this latest patch, grab me on AIM so I can go over a few things with you (provided I'm awake during your work day).

Bugs I noticed in the 04/23 experimental build on Win98 (http://ftp.mozilla.org/pub/mozilla/nightly/experimental/autocomplete/) 1. Dragging the scrollbar thumb in autocomplete dropdown: the thumb thinks my mouse cursor is ~30 pixels from where it actually is. 2. Dropdown's size isn't an integer number of sites, and mousing over a half- shown item in the outliner scrolls to make it completely visible (this feels slightly flimsy). 3. Down arrow on scrollbar moves scrollbar about .8 sites at a time instead of one. 4. Classic: Go button is misaligned. 5. Modern 3: clicking down arrow at right edge of url bar moves the down arrow (ok), but forgets to clean up a pixel of the old position of the arrow. 6. If I clear the location bar and type a hyphen (or another character that no previously-visited URLs start with), the "search for..." thing doesn't appear when I type the hyphen. 7. The Netscape logo next to "search for..." is visually distracting. 8. Typing a few random letters and then quickly pressing up or down makes those letters disappear. (In this case, the "search for..." thing hasn't appeared.) 9. Location bar context menu is gone.

I would also like to add that when I type the "ftp" part of ftp.mozilla.org, the widget only returns a "search for..." line. In the previous autocompletes, it would have matched up all the ftp places I've typed without needing the ".xxxx" part.

> 1. Dragging the scrollbar.... This is because the event.clientX/Y properties are miscalculated whenever the event target is inside of a popup. I had to file and fix bug 76297 to workaround this problem for calculating the selected row on mousemove. > 2. Dropdown's size isn't an integer number of sites... Hmmm... I used to see this problem, but I thought I had fixed it. The height of the outliner is supposed to be exactly 6 rows, and I use the newly implemented rowHeight property on nsIOutlinerBoxObject to calculate this. Perhaps it's not always giving back an accurate number. > 3. Down arrow on scrollbar moves scrollbar about .8 sites at a time... Is there a way to control how many pixels a scrollbar increments by? I'll have to look into that. > 5. Modern 3: clicking down arrow at right edge of url bar moves the down arrow (ok), but forgets to clean up a pixel of the old position of the arrow. I've seen that too. Seems to be a layout bug. > 6. If I clear the location bar... This problem has irritated me beyond believe. I am definitely telling the popup to open in that situation, however, during these cases it refuses to paint itself. It actually creates it's frames, they just don't paint. I've tried many ways to workaround this, with no success. I'm sure this will be the first major autocomplete bug filed after I land. > 7. The Netscape logo next to "search for..." is visually distracting. This isn't just the Netscape logo, it's the logo of whatever search engine you pick in your prefs under Navigator > Internet Search. Set it to Google like I do :) > 8. Typing a few random letters and then quickly pressing up or down makes those letters disappear. (In this case, the "search for..." thing hasn't appeared.) Weird, I'll try to fix that before I land. > 9. Location bar context menu is gone. Fixed that already.

>> 3. Down arrow on scrollbar moves scrollbar about .8 sites at a time... >Is there a way to control how many pixels a scrollbar increments by? I'll have >to look into that. FWIW, scrollbars for outliners in MailNews do scroll one line at a time.

I hope this is the right spot. Can we have this not show every every single file a user visits? I personally liked how it used to only display only the links that were typed in. Perhaps this could be an option that can be set? blockcipher

I agree - I find the new way too cluttered to be useful. (Not to disparage everyone's hard work or anything!)

after talking with joe over IRC, debating lots of issues, I'm proactively sr='ing this before his next patch.. there's still a leak which he has to track down, but once that is tracked down, then the sr= counts (but joe, still attach a patch here so people can see what's landing)

I thought I was noticing some leaks, just by watching memory go in the windows task manager as I typed, but alec claims not to see this on his machine, and after checking the leak logs I seem to be properly releasing all my objects. I'll post a new patch here shorly which includes post-Alec-review changes.

Not sure if anyone mentions it already, but the new autocomplete in the experimental build, like the old one, still displays URLs that lead to 404 Not Found pages. These shouldn't be recorded, since they're incorrect links, and servers should be sending a 404 header to the browser so Mozilla should be able to pick that up. The problem goes with incorrect domains, at least in the old autocomplete.

gerbilpower@yahoo.com that's covered by another bug, however the current belief (and DO NOT argue it here) is that 404 urls should be included in the autocomplete widget. Hewitt: could you color urls (or supply a different icon) by result type? perhaps a broken link icon for 404, a page w/ arrow for redirects, broken globe for dns failures, ...

if you don't like seeing 404's in autocomplete, that's really a global history problem. autocomplete just searches the global history now, so perhaps you should file a bug saying that 404 pages shouldn't be stored in history. I don't know think that http response code is stored in the history db, so there's no way for me to color code 404's.

timeless/alex, that's bug 9203, "[RFE] do not save 'dead' or incorrect url's in the location drop down".

crawl, walk, run folks... let's get hewitt's patch landed, and then we can deal with other issues as bugs. hewitt and I have talked about other changes we want to make (such as new methods for storing keywords and search queries, smarter ordering of the matches, and so forth)

ok, I'm satisfied! sr=alecf

*** Bug 71153 has been marked as a duplicate of this bug. ***

I'd like to see autocomplete-from-history as an option -- I think a lot of people prefer the use-only-what-you're-given behavior. Bug #77935

finally, fixed.

Although there are still a few kinks, I think this is an overall excellent job. Should we make a bug to get the autocomplete faster?

yes, we should open a new bug on the speed... both hewitt and I should revisit that

I would guess that any perceived speed problems would be largely in part to the slowness in creating and opening the popup, rather than the speed of searching the history.

I have to correct my previous statement. Speed is just fine for url-bar autocomplete on my celeron 550 system. However, name autocomplete when composing messages seems a lot slower. Perhaps it is because of the huge lists created with autcollect.

msg compose autocomplete is purposely slower - it's designed to not auto-complete until you stop typing.

Hmmm... this makes it so we absolutely have to pause and stop typing when we're banging out an email. I open a message, I know who I want to send it to, I type the first few letters and hit enter, but I don't get autocomplete. Me no like.

VERIFIED Fixed on all platforms with 20010511. (which of course only means that the widget is there and basically functioning as advertised, by no means have extensive tests been performed and even so would be separate bugs)