patch: khtmlimage (#70334)
Simon Hausmann
hausmann at kde.org
Tue Jan 6 11:19:32 GMT 2004
On Tuesday 06 January 2004 11:58, David Faure wrote:
> On Monday 05 January 2004 21:56, Simon Hausmann wrote:
> > Hi,
> >
> > khtmlimage has some problems figuring out the image dimensions, leading
> > to bugs like #70334. What's happening is that after the requestDone
> > signal is emitted QMovie figures out it's really done and the real pixmap
> > gets set on the RenderImage, after updateWindowCaption tried to retrieve
> > it (without success, hence the 0x0 Pixels bug) . I figured the approach
> > itself is wrong and decided to change khtml to use the cache object
> > client infrastructure instead, which allows the cache to call back on us
> > when the image really arrived. The attached patch implements this. Would
> > be cool if some kind soul could give it a second test and review it :)
>
> Is there any risk that requestImage might call notifyFinished _before_
> returning (i.e. immediately)? If this happened, then the nice window
> caption would be overwritten by the emit setWindowCaption( url.prettyURL()
> );
> in openURL.
> The fix would be easy though: just move that line up, before the
> requestImage.
Ahh, good point! Revised patch attached.
Simon
-------------- next part --------------
Index: khtmlimage.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/khtmlimage.cpp,v
retrieving revision 1.38
diff -u -p -b -r1.38 khtmlimage.cpp
--- khtmlimage.cpp 6 Jan 2004 11:11:18 -0000 1.38
+++ khtmlimage.cpp 6 Jan 2004 11:17:33 -0000
@@ -57,7 +57,7 @@ KParts::Part *KHTMLImageFactory::createP
KHTMLImage::KHTMLImage( QWidget *parentWidget, const char *widgetName,
QObject *parent, const char *name )
- : KParts::ReadOnlyPart( parent, name )
+ : KParts::ReadOnlyPart( parent, name ), m_image( 0 )
{
setInstance( KHTMLImageFactory::instance() );
@@ -111,6 +111,8 @@ KHTMLImage::KHTMLImage( QWidget *parentW
KHTMLImage::~KHTMLImage()
{
+ disposeImage();
+
// important: delete the html part before the part or qobject destructor runs.
// we now delete the htmlpart which deletes the part's widget which makes
// _OUR_ m_widget 0 which in turn avoids our part destructor to delete the
@@ -126,6 +128,8 @@ bool KHTMLImage::openURL( const KURL &ur
{
static const QString &html = KGlobal::staticQString( "<html><body><img src=\"%1\"></body></html>" );
+ disposeImage();
+
m_url = url;
emit started( 0 );
@@ -133,6 +137,8 @@ bool KHTMLImage::openURL( const KURL &ur
KParts::URLArgs args = m_ext->urlArgs();
m_mimeType = args.serviceType;
+ emit setWindowCaption( url.prettyURL() );
+
m_khtml->begin( m_url, args.xOffset, args.yOffset );
m_khtml->setAutoloadImages( true );
@@ -140,21 +146,50 @@ bool KHTMLImage::openURL( const KURL &ur
if ( impl && m_ext->urlArgs().reload )
impl->docLoader()->setCachePolicy( KIO::CC_Refresh );
+ khtml::DocLoader *dl = impl ? impl->docLoader() : 0;
+ m_image = khtml::Cache::requestImage( dl, m_url.url(), true /*reload*/ );
+ assert( m_image );
+ m_image->ref( this );
+
m_khtml->write( html.arg( m_url.url() ) );
m_khtml->end();
- emit setWindowCaption( url.prettyURL() );
-
+ /*
connect( khtml::Cache::loader(), SIGNAL( requestDone( khtml::DocLoader*, khtml::CachedObject *) ),
this, SLOT( updateWindowCaption() ) );
+ */
return true;
}
bool KHTMLImage::closeURL()
{
+ disposeImage();
return m_khtml->closeURL();
}
+void KHTMLImage::notifyFinished( khtml::CachedObject *o )
+{
+ if ( !m_image || o != m_image )
+ return;
+
+ const QPixmap &pix = m_image->pixmap();
+ QString caption;
+
+ KMimeType::Ptr mimeType;
+ if ( !m_mimeType.isEmpty() )
+ mimeType = KMimeType::mimeType( m_mimeType );
+
+ if ( mimeType )
+ caption = i18n( "%1 - %2x%3 Pixels" ).arg( mimeType->comment() )
+ .arg( pix.width() ).arg( pix.height() );
+ else
+ caption = i18n( "Image - %1x%2 Pixels" ).arg( pix.width() ).arg( pix.height() );
+
+ emit setWindowCaption( caption );
+ emit completed();
+ emit setStatusBarText(i18n("Done."));
+}
+
void KHTMLImage::guiActivateEvent( KParts::GUIActivateEvent *e )
{
// prevent the base implementation from emitting setWindowCaption with
@@ -165,6 +200,7 @@ void KHTMLImage::guiActivateEvent( KPart
KParts::ReadOnlyPart::guiActivateEvent(e);
}
+/*
void KHTMLImage::slotImageJobFinished( KIO::Job *job )
{
if ( job->error() )
@@ -225,6 +261,16 @@ void KHTMLImage::updateWindowCaption()
emit completed();
emit setStatusBarText(i18n("Done."));
}
+*/
+
+void KHTMLImage::disposeImage()
+{
+ if ( !m_image )
+ return;
+
+ m_image->deref( this );
+ m_image = 0;
+}
bool KHTMLImage::eventFilter(QObject *, QEvent *e) {
switch (e->type()) {
Index: khtmlimage.h
===================================================================
RCS file: /home/kde/kdelibs/khtml/khtmlimage.h,v
retrieving revision 1.8
diff -u -p -b -r1.8 khtmlimage.h
--- khtmlimage.h 30 Aug 2003 16:18:29 -0000 1.8
+++ khtmlimage.h 6 Jan 2004 11:17:33 -0000
@@ -24,9 +24,16 @@
#include <kparts/factory.h>
#include <kparts/browserextension.h>
+#include "misc/loader_client.h"
+
class KHTMLPart;
class KInstance;
+namespace khtml
+{
+ class CachedImage;
+};
+
/**
* @internal
*/
@@ -50,7 +57,7 @@ private:
/**
* @internal
*/
-class KHTMLImage : public KParts::ReadOnlyPart
+class KHTMLImage : public KParts::ReadOnlyPart, public khtml::CachedObjectClient
{
Q_OBJECT
public:
@@ -66,19 +73,24 @@ public:
KHTMLPart *doc() const { return m_khtml; }
+ virtual void notifyFinished( khtml::CachedObject *o );
+
protected:
virtual void guiActivateEvent( KParts::GUIActivateEvent *e );
virtual bool eventFilter( QObject *filterTarget, QEvent *e );
private slots:
- void slotImageJobFinished( KIO::Job *job );
+// void slotImageJobFinished( KIO::Job *job );
- void updateWindowCaption();
+// void updateWindowCaption();
private:
+ void disposeImage();
+
QGuardedPtr<KHTMLPart> m_khtml;
KParts::BrowserExtension *m_ext;
QString m_mimeType;
+ khtml::CachedImage *m_image;
};
/**
More information about the kfm-devel
mailing list