Bug 116181

Summary: Getting cover from internet doesn't work
Product: juk Reporter: Hasso Tepper <hasso>
Component: generalAssignee: Scott Wheeler <wheeler>
Status: RESOLVED FIXED    
Severity: normal CC: faure
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: repair google fetch covers

Description Hasso Tepper 2005-11-12 14:09:12 UTC
Version:           2.3 (using KDE 3.5 (RC1), compiled sources)
Compiler:          Target: i486-linux-gnu
OS:                Linux (i686) release 2.6.13.4-rt14

3.5 branch recompiled today morning. Getting cover from internet doesn't work at all. With debug turned on I see this in konsole:

juk: Performing Google Search: http://images.google.com/images?q=Abydos%20Abydos
ASSERT: "firstBody" in html_tableimpl.cpp (316)
Comment 1 Michael Pyne 2005-11-12 23:26:56 UTC
I can get the ASSERT here as well, with the exception that it doesn't actually crash JuK for some reason (and the feature still works).  This code was committed for bug 105586, maybe dfaure knows more?

juk: Performing Google Search: http://images.google.com/images?q=Israfel%20Kong%20in%20Concert
ASSERT: "firstBody" in /home/kde-cvs/kdesvn/kdelibs/khtml/html/html_tableimpl.cpp (316)
kparts: 0x8742d50 emitting activePartChanged (nil)
kio (KIOJob): Job::kill this=0x876c9a8 KIO::TransferJob m_progressId=0 quietly=true
kio (KIOJob): Job::kill this=0x876c090 KIO::TransferJob m_progressId=0 quietly=true
kio (Scheduler): Scheduler: killing slave 22431
kio (Slave): killing slave pid=22431 (http://images.google.com)
kio (KIOJob): Job::kill this=0x876ad20 KIO::TransferJob m_progressId=0 quietly=true
kio (Scheduler): Scheduler: killing slave 22437
kio (Slave): killing slave pid=22437 (http://images.google.com)
kio (KIOJob): Job::kill this=0x876b838 KIO::TransferJob m_progressId=0 quietly=true
kparts: Part::~Part 0xbffc6dc0
kparts: deleting widget [KHTMLView pointer (0x872aa68) to unnamed widget, geometry=640x409+320+307] unnamed
Comment 2 S.Çağlar ONUR 2005-11-26 09:18:54 UTC
same situation here with 3.5.0_rc2 [ASSERT: "firstBody" in html_tableimpl.cpp (316)] 

(and also juk crashes on exit with *** glibc detected *** corrupted double-linked list: 0x08227b70 *** most probably because of assetion error)
Comment 3 David Faure 2005-11-28 10:56:43 UTC
I don't get this Q_ASSERT in konqueror...
Comment 4 Michael Pyne 2005-11-29 00:13:18 UTC
Nor do I.

The relevant KHTML code is in kdemultimedia/juk/googlefetcher.cpp:90 if you'd like to take a look.

It's rather unconventional usage of KHTML to be sure, but I'd rather not have to parse the HTML myself, and I don't want to have a show() a widget just to get to the information.

Note that I'm not even sure if doing a show() would even work. :(
Comment 5 Seb Ruiz 2005-11-29 07:43:37 UTC
We had a nasty bug that would crash amarok if kio was passed an empty url, or killed when there was no url.  Perhaps it is something related to that?

Here is a commit link:
http://svntalk.pwsp.net/project/amaroK/revision/481676

I haven't looked at your code, so I'm just stabbing in the dark.
Comment 6 Ismail Donmez 2005-11-29 12:12:55 UTC
I got the same assert but no crash.
Comment 7 Hasso Tepper 2006-02-26 10:11:47 UTC
SVN commit 513732 by hasso:

Don't punish non-english users and make getting cover from Internet working
again for them - run search always in english. I will not close #116181
though because mentioned asserts are still there.

CCMAIL:kde-et@linux.ee
CCBUG:116181


 M  +1 -0      googlefetcher.cpp  


--- branches/KDE/3.5/kdemultimedia/juk/googlefetcher.cpp #513731:513732
@@ -58,6 +58,7 @@
 
     KURL url("http://images.google.com/images");
     url.addQueryItem("q", m_searchString);
+    url.addQueryItem("hl", "en");
     
     switch (size) {
         case XLarge:
Comment 8 Michal Bukovsky 2006-10-18 08:49:14 UTC
hi,
this feature don't work for me. I found some bug.

First google fill page with javascript, there is option 'nojs' which disable javascript.
Second in url of images is now http:// prefix.
Comment 9 Michal Bukovsky 2006-10-18 08:56:45 UTC
Created attachment 18157 [details]
repair google fetch covers
Comment 10 Michal Bukovsky 2006-10-18 08:59:18 UTC
I forgot, konqueror need to be set no send identification, because google try detect javascript....
Comment 11 Michael Pyne 2007-02-01 05:35:30 UTC
SVN commit 628971 by mpyne:

Disable Google's javascript when loading images for the cover search, and make
sure that any images we try to load have only one "http://" protocol header.
Based on a patch from Michal Bukovsky.

I don't get messages about exceptions right now but I also don't have kdelibs built
with debugging so I'm leaving the bug open for now.  I will forward port tomorrow.

CCBUG:116181


 M  +13 -7     googlefetcher.cpp  
 M  +3 -3      main.cpp  


--- branches/KDE/3.5/kdemultimedia/juk/googlefetcher.cpp #628970:628971
@@ -1,6 +1,6 @@
 /***************************************************************************
-    copyright            : (C) 2004 Nathan Toone
-    email                : nathan@toonetown.com
+    copyright            : (C) 2004 Nathan Toone <nathan@toonetown.com>
+    copyright            : (C) 2007 Michael Pyne <michael.pyne@kdemail.com>
 ***************************************************************************/
 
 /***************************************************************************
@@ -37,7 +37,12 @@
     // thumbURL is in the following format - and we can regex the imageURL
     // images?q=tbn:hKSEWNB8aNcJ:www.styxnet.com/deyoung/styx/stygians/cp_portrait.jpg
 
-    m_imageURL = "http://" + thumbURL.remove(QRegExp("^.*q=tbn:[^:]*:"));
+    m_imageURL = thumbURL.remove(QRegExp("^.*q=tbn:[^:]*:"));
+
+    // Ensure that the image url starts with http if it doesn't already.
+    if(!m_imageURL.startsWith("http://"))
+        m_imageURL.prepend("http://");
+
     m_size = size.replace("pixels - ", "\n(") + ")";
 }
 
@@ -59,7 +64,8 @@
     KURL url("http://images.google.com/images");
     url.addQueryItem("q", m_searchString);
     url.addQueryItem("hl", "en");
-    
+    url.addQueryItem("nojs", "1");
+
     switch (size) {
         case XLarge:
             url.addQueryItem("imgsz", "xlarge|xxlarge");
@@ -79,7 +85,7 @@
         default:
             break;
     }
-    
+
     m_loadedQuery = m_searchString;
     m_loadedSize = size;
 
@@ -139,7 +145,7 @@
 	DOM::NodeList images = rows.item(imageIndex).childNodes();
 
 	// For each table node, pull the images out of the first row
-	    
+
 	for(uint j = 0; j < images.length(); j++) {
 	    DOM::Element tdElement = images.item(j);
 	    if(tdElement.isNull()) {
@@ -246,7 +252,7 @@
 bool GoogleFetcher::hasImageResults(DOM::HTMLDocument &search)
 {
     unsigned long typesToShow = DOM::NodeFilter::SHOW_TEXT;
-    
+
     DOM::NodeIterator it = search.createNodeIterator(search.body(), typesToShow, 0, false);
     DOM::Node node;
 
--- branches/KDE/3.5/kdemultimedia/juk/main.cpp #628970:628971
@@ -1,7 +1,7 @@
 /***************************************************************************
     begin                : Mon Feb  4 23:40:41 EST 2002
     copyright            : (C) 2002 - 2004 by Scott Wheeler
-                         : (C) 2006 by Michael Pyne
+                         : (C) 2006 - 2007 by Michael Pyne
     email                : wheeler@kde.org
                          : michael.pyne@kdemail.net
 ***************************************************************************/
@@ -50,8 +50,8 @@
 int main(int argc, char *argv[])
 {
     KAboutData aboutData("juk", I18N_NOOP("JuK"),
-                         "2.3.3", description, KAboutData::License_GPL,
-                         "© 2002 - 2006, Scott Wheeler", 0,
+                         "2.3.4", description, KAboutData::License_GPL,
+                         "© 2002 - 2007, Scott Wheeler", 0,
                          "http://developer.kde.org/~wheeler/juk.html");
 
     aboutData.addAuthor("Scott Wheeler", scott, "wheeler@kde.org");
Comment 12 Michael Pyne 2007-02-03 00:01:23 UTC
SVN commit 629507 by mpyne:

Forward port potential fix for bug 116181 to KDE 4, a day late.

It's in the process of compiling so hopefully the new QString doesn't
break the build. :)

CCBUG:116181


 M  +9 -3      googlefetcher.cpp  


--- trunk/KDE/kdemultimedia/juk/googlefetcher.cpp #629506:629507
@@ -1,6 +1,6 @@
 /***************************************************************************
-    copyright            : (C) 2004 Nathan Toone
-    email                : nathan@toonetown.com
+    copyright            : (C) 2004 Nathan Toone <nathan@toonetown.com>
+    copyright            : (C) 2007 Michael Pyne <michael.pyne@kdemail.com>
 ***************************************************************************/
 
 /***************************************************************************
@@ -40,7 +40,12 @@
     // thumbURL is in the following format - and we can regex the imageURL
     // images?q=tbn:hKSEWNB8aNcJ:www.styxnet.com/deyoung/styx/stygians/cp_portrait.jpg
 
-    m_imageURL = "http://" + thumbURL.remove(QRegExp("^.*q=tbn:[^:]*:"));
+    m_imageURL = thumbURL.remove(QRegExp("^.*q=tbn:[^:]*:"));
+
+    // Ensure that the image url starts with http if it doesn't already.
+    if(!m_imageURL.startsWith("http://"))
+        m_imageURL.prepend("http://");
+
     m_size = size.replace("pixels - ", "\n(") + ')';
 }
 
@@ -62,6 +67,7 @@
     KUrl url("http://images.google.com/images");
     url.addQueryItem("q", m_searchString);
     url.addQueryItem("hl", "en");
+    url.addQueryItem("nojs", "1");
 
     switch (size) {
         case XLarge:
Comment 13 Michael Pyne 2007-06-21 03:57:23 UTC
This is fixed in the 3.5 branch and KDE 4 as of revision 678286 by replacing the image search with a search engine which is designed to be able to be searched programatically (Yahoo Image Search at this point).