Version: (using KDE KDE 3.5.4) Installed from: Compiled From Sources Compiler: GCC 3.4.6 -march=athlon-tbird OS: Linux When highlighting text, right mouse click -> search at google, if the text selected contains and ampersand, the text passed (parsed) for input is cut off at the ampersand; Select the text below, right mouse click -> search at google: KDE & Konqueror KDE&Konqueror Nick
The & is not converted to %26, but copied as is. This works for other searching places in konqueror, so it seems related to the rightclick search only.
The error probably is in kdelibs/khtml/khtml_ext.cpp: KHTMLPopupGUIClient::KHTMLPopupGUIClient (near line 400). a) The search text cannot be taken into the menu as is. 1) Selecting & in text results in interpretation as hotkey. 2) Selecting e.g. the text %26 above results in wrong text building, as %2 is interpreted as printf style marker b) Finally URL encoding seems to be missing. P.S. Sorry, but I'm not willing to debug kdelibs ATM.
OK, thanks for looking at this. I got it so the 'menu' entry reads correct, but the stringe is still passed wrongly: kdelibs/khtml/khtml_ext.cpp around line 444 // search text QString selectedText = khtml->selectedText(); +selectedText.replace("&", "&&"); if ( selectedText.length()>18 ) { selectedText.truncate(15); selectedText+="..."; } But it's no big deal anyway... be just nice to fix next time around. Nick
Marked as Junior Job. I think this should be pretty easy to fix.
---------- Forwarded message ---------- From: Will Entriken <kde.org@phor.net> Date: Aug 27, 2006 11:46 AM Subject: [patch] bug 132678 To: 132678@bugzilla.kde.org, kde-devel <kde-devel@kde.org> I think I fixed this bug. It seems like there is a mix of escaped and unescaped query strings, this fixes that. I would commit this patch myself, but it could takes days for my working copy to compile. Updating and compiling really sucks. Created an attachment (id=17521) 132678.patch
I can't this to build: khtml_ext.cpp: In member function `void KHTMLPartBrowserExtension::searchProvider()': khtml_ext.cpp:276: error: incomplete type `QUrl' used in nested name specifier make[1]: *** [libkhtml_la.all_cpp.lo] Error 1 make[1]: Leaving directory `/home/hdb/kde_build/konstruct/kde/kdelibs/work/kdelibs-3.5.4/khtml' Nick ?
SVN commit 577831 by entriken: CCBUG: 132678 It worksforme, but this patch may fix Nick's problem at: http://bugs.kde.org/show_bug.cgi?id=132678#c6 M +1 -0 khtml_ext.cpp --- trunk/KDE/kdelibs/khtml/khtml_ext.cpp #577830:577831 @@ -38,6 +38,7 @@ #include <qclipboard.h> #include <qfileinfo.h> #include <qmenu.h> +#include <qurl.h> #include <qmetaobject.h> #include <kdebug.h>
I don't even have the same headers included in khtml_ext.cpp (3.5.4): #include "html/html_imageimpl.h" #include "misc/loader.h" #include "dom/html_form.h" #include "dom/html_image.h" #include <qclipboard.h> #include <qfileinfo.h> #include <qpopupmenu.h> ???????????? #include <qmetaobject.h> #include <private/qucomextra_p.h> #include <qdragobject.h> #include <kdebug.h> #include <klocale.h> #include <kfiledialog.h>
OK, if I do add in the additional qurl.h header, I get a different build issue: In file included from libkhtml_la.all_cpp.cpp:7: khtml_ext.cpp: In member function `void KHTMLPartBrowserExtension::searchProvider()': khtml_ext.cpp:277: error: `toPercentEncoding' is not a member of `QUrl' make[1]: *** [libkhtml_la.all_cpp.lo] Error 1 make[1]: Leaving directory `/home/hdb/kde_build/konstruct/kde/kdelibs/work/kdelibs-3.5.4/khtml' Nick
BUGGER ME! I fixed this patch. After reading QT docs, and reading build errors: #include <qclipboard.h> #include <qfileinfo.h> #include <qmenu.h> +#include <qurl.h> #include <qmetaobject.h> if( !KURIFilter::self()->filterURI(data, list) ) { KDesktopFile file("searchproviders/google.desktop", true, "services"); + QString encodedSearchTerm = m_part->selectedText(); + QUrl::encode(encodedSearchTerm); data.setData(file.readEntry("Query").replace("\\{@}", encodedSearchTerm)); // search text QString selectedText = khtml->selectedText(); +selectedText.replace("&", "&&"); if ( selectedText.length()>18 ) { selectedText.truncate(15); selectedText+="..."; } I apologise for not submitting a proper patch... Thanks, Will ! Nick
Created attachment 17524 [details] Patch against KDE3.5.4 khtml_ext.cpp This works fine... patch attached.
SVN commit 579136 by stoecker: CCBUG: 132678 Backport of Google search encoding fix for KDE 3.5 by Nick Warne. M +5 -1 khtml_ext.cpp --- branches/KDE/3.5/kdelibs/khtml/khtml_ext.cpp #579135:579136 @@ -38,6 +38,7 @@ #include <qclipboard.h> #include <qfileinfo.h> #include <qpopupmenu.h> +#include <qurl.h> #include <qmetaobject.h> #include <private/qucomextra_p.h> #include <qdragobject.h> @@ -273,7 +274,9 @@ if( !KURIFilter::self()->filterURI(data, list) ) { KDesktopFile file("searchproviders/google.desktop", true, "services"); - data.setData(file.readEntry("Query").replace("\\{@}", m_part->selectedText())); + QString encodedSearchTerm = m_part->selectedText(); + QUrl::encode(encodedSearchTerm); + data.setData(file.readEntry("Query").replace("\\{@}", encodedSearchTerm)); } KParts::URLArgs args; @@ -441,6 +444,7 @@ // search text QString selectedText = khtml->selectedText(); + selectedText.replace("&", "&&"); if ( selectedText.length()>18 ) { selectedText.truncate(15); selectedText+="...";
The reported bug is fixed, but selecting "test%2" still results in an error based on the i18n functionality. Needs to be fixed as well. May also be a QT bug.
Ummm, yes - %1 (or %10, %11 etc.) breaks this also. I think the offending line is 480: new KAction( i18n( "Search '%1' at %2" ).arg( selectedText ).arg( name ), icon, 0, d->m_khtml->browserExtension(), SLOT( searchProvider() ), actionCollection(), "searchProvider" ); Starting to get deep... Nick
Created attachment 17681 [details] Dirty hack to fix the percent issues . Real dirty hack to fix this percent issue - I think to fix this properly, a lot of diving into the code needs to done. I can't see how both %1 and %2 get 'parsed' as argv in new Kaction if present in SelectedText? - new KAction( i18n( "Search '%1' at %2" ).arg( selectedText ).arg( name ), icon, 0, d->m_khtml->browserExtension(), + // Dirty hack to stop %1 and %2 being being somehow parsed as argv in new Kaction when + // appearing in SelectedText - see bug 132678 + QString selectedTextandName = "'" + selectedText + "' at " + name; + //new KAction( i18n( "Search '%1' at %2" ).arg( selectedText ).arg( name ), icon, 0, d->m_khtml->browserExtension(), + new KAction( i18n( "Search %1" ).arg( selectedTextandName ), icon, 0, d->m_khtml->browserExtension(), SLOT( searchProvider() ), actionCollection(), "searchProvider" ); Nick
Actually, this is a very deep issue. %26 %27 %28 Will appear correct now in right mouse click menu, but actual data sent to google (etc.) is the encoded characters [ & ' ( ]. So, with the two patches I supplied, it is better and partially fixed, but this underlying issue runs deep. Nick
Sorry Nick, this does not work. The types %1 and %2 are used, because other languages may require other orders of the arguments. The problem here is that for i18n("Hello %1 %2").arg(x1).arg(x2) the parsing should not reparse the included text (x1) when scanning for %2. Actually it is a QT bug and I will report it there and close this one. #include <QApplication> #include <QPushButton> int main(int argc, char *argv[]) { QApplication app(argc, argv); QPushButton hello(QString("1:%1 2:%2 3:%3").arg("%1").arg("%2").arg("test3")); hello.resize(200, 50); hello.show(); return app.exec(); } Resolving as fixed, as the main bug report has been fulfilled.
SVN commit 583923 by stoecker: Fixed text error for certain selected texts. CCBUG: 132678 M +2 -1 khtml_ext.cpp --- branches/KDE/3.5/kdelibs/khtml/khtml_ext.cpp #583922:583923 @@ -476,7 +476,8 @@ name = "Google"; } - new KAction( i18n( "Search for '%1' with %2" ).arg( selectedText ).arg( name ), icon, 0, d->m_khtml->browserExtension(), + // using .arg(foo, bar) instead of .arg(foo).arg(bar), as foo can contain %x + new KAction( i18n( "Search for '%1' with %2" ).arg( selectedText, name ), icon, 0, d->m_khtml->browserExtension(), SLOT( searchProvider() ), actionCollection(), "searchProvider" ); // favorite search providers
Hi Dirk, Heh... so simple a fix. Thanks, all appears to work as it should here! Good job! Nick