One other thing, it is easier codewise to simply not strip the %s for normal bookmark access. I suppose custom keyword bookmarks can be kept in a separate folder.

I suppose custom keyword bookmarks can be kept in a separate folder.

That's how they are kept in Mozilla by default.

Here is an implementation of bookmark keywords substitution (%s) that should work the same as in Mozilla:

BrowserView.cpp (bug 289 - bookmark keywords)
---------------

#include "kmeleon_plugin.h"
#include <wininet.h>


void CBrowserView::OnNewUrlEnteredInUrlBar()
{
char *p, *q, *r;
char nickUrl[INTERNET_MAX_URL_LENGTH];
char custUrl[INTERNET_MAX_URL_LENGTH];

mpBrowserFrame->m_wndUrlBar.EditChanged(FALSE);

nsCOMPtr<nsIWebBrowserFocus> focus(do_GetInterface(mWebBrowser));
if(focus)
focus->Activate();

// Get the currently entered URL
CString strUrl;
mpBrowserFrame->m_wndUrlBar.GetEnteredURL(strUrl);

// Check for a nickname
*nickUrl = 0;
p = strUrl.GetBuffer(0); // get entered URL
p = SkipWhiteSpace(p); // skip any leading spaces
q = strchr(p, ' '); // look for a space
if (q) // if more than one word ...
*q = 0; // terminate first word
theApp.plugins.SendMessage("bookmarks", "* FindNick", "FindNick", (long)p, (long)&nickUrl);
if (q) // if more than one word
*q = ' '; // restore space
if (*nickUrl != 0) {
r = strstr(nickUrl, "%s"); // look for %s
if (r) { // if found ...
*r = 0; // terminate string up to %s
strcpy(custUrl, nickUrl); // copy string before %s
strcat(custUrl, q+1); // copy second word
strcat(custUrl, r+2); // copy string after %s
OpenURL(custUrl);
}
else
OpenURL(nickUrl);
}
else {
theApp.plugins.SendMessage("hotlist", "* FindNick", "FindNick", (long)p, (long)&nickUrl);
if (*nickUrl != 0)
OpenURL(nickUrl);
else {
if(IsViewSourceUrl(strUrl))
OpenViewSourceWindow(strUrl.GetBuffer(0));
else
// Navigate to that URL
OpenURL(strUrl.GetBuffer(0));
}
}

// Add what was just entered into the UrlBar
mpBrowserFrame->m_wndUrlBar.AddURLToList(strUrl);
}

asmpgmr:

Yup, that works.

Binary same place as usual. Installer packages being updated right now.

Good stuff.


Shelumi`El
Jordan

S.D.G

MonkeeSage,

That doesn't work for me, I get the error message "The URL is not valid and cannot be loaded" for custom keywords with parameters and I also got a crash once. Back that change out and restore the original bookmark keywords code.

asmpgmr:

I think I was missing a bracket in the 1.,2.1 build (missed it when I copied from the 1.3 code). Give it another try, and if it still doesn't work I'll revert to the code from your last email.


Shelumi`El
Jordan

S.D.G

MonkeeSage,

The new compile was exactly the same. Anyway it seems unstable, sometimes it works and sometimes it doesn't. It should work since I'm pretty sure the code logic is correct. The only possible issue I can see is that I'm directly modifying a "cstring" (*q = 0 and *q = ' ') and maybe you're not supposed to do that ? Why can't c++ always use normal char strings, stupid object-oriented language. Since I'm not sure what's going on I would prefer this be backed out and the original bookmark keywords code fix (without %s) restored which always worked.

Hmm....Works like a charm for me. No crashes at all or inconsistencies regarding normal keyword function or quicksearch(%s) function.

This was unstable for me and I must insist that this be backed out. I do not want to be responsible for putting unstable code out there.

Anybody else trying it out besides us 3?

IRRELEVANT, I got three different results when I tested it. I didn't expect it to not work or I wouldn't have posted it but since it doesn't I want it removed and the original code which worked restored. If I figure something out then I'll post an update but for now I absolutely insist that this be removed.

asmpgmr:

If you like, I'll build you a binary w/o it; but since it does work as expected for me and I've had no problems (and works on my brother's computer the same), and it works as expected for jsnj, I'm going to keep it in the builds in my installers. It even works on eccentric stuff for me, like this:

Bookmark: http://kmeleon.source%s
Nick: kmh

In URL bar: kmh forge.net
Result: http://kmeleon.sourceforge.net opens.


Shelumi`El
Jordan

S.D.G

MonkeeSage,

I have both versions. I always keep the previous version until I verify that the latest version works without problems.

I looked over the code and I don't see any reason why it shouldn't work and I just retested the new version and it seems to work now but it was giving inconsistent results last night - first a crash then the URL is not valid error. I'm not comfortable with it because something was going on.

Found what was causing the crash, an if (q) is needed before the first strcat:

BrowserView.cpp
---------------

#include "kmeleon_plugin.h"
#include <wininet.h>


void CBrowserView::OnNewUrlEnteredInUrlBar()
{
char *p, *q, *r;
char nickUrl[INTERNET_MAX_URL_LENGTH];
char custUrl[INTERNET_MAX_URL_LENGTH];

mpBrowserFrame->m_wndUrlBar.EditChanged(FALSE);

nsCOMPtr<nsIWebBrowserFocus> focus(do_GetInterface(mWebBrowser));
if(focus)
focus->Activate();

// Get the currently entered URL
CString strUrl;
mpBrowserFrame->m_wndUrlBar.GetEnteredURL(strUrl);

// Check for a nickname
*nickUrl = 0;
p = strUrl.GetBuffer(0); // get entered URL
p = SkipWhiteSpace(p); // skip any leading spaces
q = strchr(p, ' '); // look for a space
if (q) // if more than one word
*q = 0; // terminate first word
theApp.plugins.SendMessage("bookmarks", "* FindNick", "FindNick", (long)p, (long)&nickUrl);
if (q) // if more than one word
*q = ' '; // restore space
if (*nickUrl != 0) {
r = strstr(nickUrl, "%s"); // look for %s
if (r) { // if found
*r = 0; // terminate string up to %s
strcpy(custUrl, nickUrl); // copy string before %s
if (q) // if more than one word
strcat(custUrl, q+1); // copy second word
strcat(custUrl, r+2); // copy string after %s
OpenURL(custUrl);
}
else
OpenURL(nickUrl);
}
else {
theApp.plugins.SendMessage("hotlist", "* FindNick", "FindNick", (long)p, (long)&nickUrl);
if (*nickUrl != 0)
OpenURL(nickUrl);
else {
if(IsViewSourceUrl(strUrl))
OpenViewSourceWindow(strUrl.GetBuffer(0));
else
// Navigate to that URL
OpenURL(strUrl.GetBuffer(0));
}
}

// Add what was just entered into the UrlBar
mpBrowserFrame->m_wndUrlBar.AddURLToList(strUrl);
}

asmpgmr:

Here is the build with the updated code:

http://monkeesage.d2g.com/k-meleon.exe


Shelumi`El
Jordan

S.D.G

No problems now, the additional if statement fixed it so it's good to go.

MonkeeSage and jsnj,

Does it work as you expect it to ? The Mozilla bookmark keywords handling should be the same as Mozilla. Currently the favorites plugin doesn't have nickname support so in the main program code I submitted to the devs, the nickname lookup (with %s) is there but commented out. Apparently with some browser add-on pack you get a function in IE (quicksearch) which is basically the same as bookmark keywords. As far as I know Opera only support nicknames with no substitution feature. They handle custom searches in a completely different manner.

So what's there now:
bookmarks - nicknames with %s substitution
favorites - no nicknames (once the plugin is updated nicknames with %s substitution)
hotlist - nicknames only

For me, the new code works exactly the same as the first one you submitted, without a problem. The quicksearch add-on for IE 5.5 doesn't show up in KM's favorites plugin because it's not designed as a favorite or bookmark per se but as an exe only visible in the favorites list through IE even though the design is using the "%s" method.

asmpgmr:

It works well for me with nothing unexpected happening.


Shelumi`El
Jordan

S.D.G