Bug 132678

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