SVN commit 788539 by dfaure: The fact that text/html derives from text/plain brought back a bug that I fixed in 2002: "embed katepart and then type a website URL -> loaded into katepart". The test for mimetype inheritance in changeViewMode came from bug #108542, but that was wrong [the current mimetype shouldn't matter, only what the view supports, otherwise viewing a text/plain file or a C++ file in katepart changes what happens when opening a text/html file later on]. ... and in kde4 it should be different anyway with dolphinpart handling all main viewmodes. M +2 -2 KonqViewAdaptor.cpp M +2 -2 KonqViewAdaptor.h M +22 -11 konqmainwindow.cpp M +29 -27 konqview.cpp M +20 -12 konqview.h M +5 -0 tests/CMakeLists.txt M +0 -2 tests/konqhtmltest.cpp A tests/konqviewtest.cpp [License: GPL (v2+)] --- trunk/KDE/kdebase/apps/konqueror/src/KonqViewAdaptor.cpp #788538:788539 @@ -35,10 +35,10 @@ m_pView->openUrl( KUrl(url), locationBarURL, nameFilter ); } -bool KonqViewAdaptor::changeViewMode( const QString &serviceType, +bool KonqViewAdaptor::changeViewMode( const QString &mimeType, const QString &serviceName ) { - return m_pView->changeViewMode( serviceType, serviceName ); + return m_pView->changePart( mimeType, serviceName ); } void KonqViewAdaptor::lockHistory() --- trunk/KDE/kdebase/apps/konqueror/src/KonqViewAdaptor.h #788538:788539 @@ -56,11 +56,11 @@ /** * Change the type of view (i.e. loads a new konqueror view) - * @param serviceType the service type we want to show + * @param mimeType the mime type we want to show * @param serviceName allows to enforce a particular service to be chosen, * @see KonqFactory. */ - bool changeViewMode( const QString &serviceType, + bool changeViewMode( const QString &mimeType, const QString &serviceName ); /** --- trunk/KDE/kdebase/apps/konqueror/src/konqmainwindow.cpp #788538:788539 @@ -478,7 +478,7 @@ // #4070: Give focus to view after URL was entered manually // Note: we do it here if the view mode (i.e. part) wasn't changed - // If it is changed, then it's done in KonqView::changeViewMode + // If it is changed, then it's done in KonqView::changePart if ( m_currentView && m_currentView->part() ) m_currentView->part()->widget()->setFocus(); @@ -745,7 +745,7 @@ // In case we open an index.html, we want the location bar // to still display the original URL (so that 'up' uses that URL, // and since that's what the user entered). - // changeViewMode will take care of setting and storing that url. + // changePart will take care of setting and storing that url. QString originalURL = url.pathOrUrl(); if ( !req.nameFilter.isEmpty() ) // keep filter in location bar { @@ -783,14 +783,14 @@ KConfig config(urlDotDir.path(), KConfig::SimpleConfig); KConfigGroup urlProperties( &config, "URL properties" ); HTMLAllowed = urlProperties.readEntry( "HTMLAllowed", m_bHTMLAllowed); - serviceName = urlProperties.readEntry( "ViewMode", serviceName ); + //serviceName = urlProperties.readEntry( "ViewMode", serviceName ); //kDebug(1202) << "serviceName=" << serviceName; } if ( HTMLAllowed && ( !( indexFile = findIndexFile( url.path() ) ).isEmpty() ) ) { mimeType = "text/html"; url = KUrl(indexFile); - serviceName.clear(); // cancel what we just set, this is not a dir finally + //serviceName.clear(); // cancel what we just set, this is not a dir finally } // Reflect this setting in the menu @@ -877,8 +877,19 @@ return true; // handled } } - if ( ok ) - ok = childView->changeViewMode( mimeType, serviceName, forceAutoEmbed ); + if ( ok ) { + + // When typing a new URL, the current context doesn't matter anymore + // -> select the preferred part for a given mimetype (even if the current part can handle this mimetype). + // This fixes the "get katepart and then type a website URL -> loaded into katepart" problem + // (first fixed in r168902 from 2002!, see also unittest KonqHtmlTest::textThenHtml()) + + if (!req.typedUrl.isEmpty() || !serviceName.isEmpty()) { + ok = childView->changePart( mimeType, serviceName, forceAutoEmbed ); + } else { + ok = childView->ensureViewSupports( mimeType, forceAutoEmbed ); + } + } } } @@ -1552,7 +1563,7 @@ void KonqMainWindow::slotViewModeTriggered(QAction* action) { - // Gather data from the action, since the action will be deleted by changeViewMode + // Gather data from the action, since the action will be deleted by changePart const QString modeName = action->objectName(); const QString internalViewMode = action->data().toString(); @@ -1560,7 +1571,7 @@ m_currentView->stop(); m_currentView->lockHistory(); - // Save those, because changeViewMode will lose them + // Save those, because changePart will lose them KUrl url = m_currentView->url(); QString locationBarURL = m_currentView->locationBarURL(); #if 0 @@ -1576,7 +1587,7 @@ } #endif - m_currentView->changeViewMode( m_currentView->serviceType(), modeName ); + m_currentView->changePart( m_currentView->serviceType(), modeName ); KUrl locURL( locationBarURL ); QString nameFilter = detectNameFilter( locURL ); #if 0 @@ -4717,8 +4728,8 @@ m_currentView->stop(); m_currentView->setLocationBarURL(m_popupUrl); m_currentView->setTypedURL(QString()); - if ( m_currentView->changeViewMode( m_popupMimeType, - service->desktopEntryName() ) ) + if ( m_currentView->changePart( m_popupMimeType, + service->desktopEntryName(), true ) ) m_currentView->openUrl( m_popupUrl, m_popupUrl.pathOrUrl() ); } --- trunk/KDE/kdebase/apps/konqueror/src/konqview.cpp #788538:788539 @@ -304,37 +304,39 @@ } } -bool KonqView::changeViewMode( const QString &mimeType, - const QString &serviceName, - bool forceAutoEmbed ) +bool KonqView::ensureViewSupports( const QString &mimeType, + bool forceAutoEmbed ) { - // Caller should call stop first. - assert ( !m_bLoading ); + if (supportsMimeType(mimeType)) + return true; + return changePart(mimeType, QString(), forceAutoEmbed); +} - kDebug(1202) << "mimeType is" << mimeType - << "serviceName is" << serviceName - << "current service name is" << m_service->desktopEntryName(); +bool KonqView::changePart(const QString &mimeType, + const QString &serviceName, + bool forceAutoEmbed) +{ + // Caller should call stop first. + assert( !m_bLoading ); - KMimeType::Ptr mime = KMimeType::mimeType(mimeType); - if (!mime) // huh? - return false; + //kDebug(1202) << "mimeType=" << mimeType + // << "requested serviceName=" << serviceName + // << "current service name=" << m_service->desktopEntryName(); - // See bug #108542 - if (mime->is(m_serviceType) && (serviceName.isEmpty() || serviceName == m_service->desktopEntryName())) { - return true; - } + if (serviceName == m_service->desktopEntryName()) { + m_serviceType = mimeType; + return true; + } - if ( isLockedViewMode() ) - { - //kDebug(1202) << "This view's mode is locked - can't change"; - return false; // we can't do that if our view mode is locked - } + if (isLockedViewMode()) { + //kDebug(1202) << "This view's mode is locked - can't change"; + return false; // we can't do that if our view mode is locked + } - kDebug(1202) << "Switching view modes..."; - KService::List partServiceOffers, appServiceOffers; - KService::Ptr service; - KonqFactory konqFactory; - KonqViewFactory viewFactory = konqFactory.createView( mimeType, serviceName, &service, &partServiceOffers, &appServiceOffers, forceAutoEmbed ); + KService::List partServiceOffers, appServiceOffers; + KService::Ptr service; + KonqFactory konqFactory; + KonqViewFactory viewFactory = konqFactory.createView( mimeType, serviceName, &service, &partServiceOffers, &appServiceOffers, forceAutoEmbed ); if ( viewFactory.isNull() ) { @@ -349,7 +351,7 @@ m_appServiceOffers = appServiceOffers; // Check if that's already the kind of part we have -> no need to recreate it - // Note: we should have an operator= for KService... + // Note: we should have an operator== for KService... if ( m_service && m_service->entryPath() == service->entryPath() ) { kDebug( 1202 ) << "Reusing service. Service type set to" << m_serviceType; @@ -829,7 +831,7 @@ setPageSecurity( h.pageSecurity ); m_sTypedURL.clear(); - if (!changeViewMode(h.strServiceType, h.strServiceName)) { + if (!changePart(h.strServiceType, h.strServiceName)) { kWarning(1202) << "Couldn't change view mode to" << h.strServiceType << h.strServiceName; return /*false*/; } --- trunk/KDE/kdebase/apps/konqueror/src/konqview.h #788538:788539 @@ -105,18 +105,26 @@ const QString &nameFilter = QString(), bool tempFile = false ); - /** - * Change the type of view (i.e. loads a new konqueror view) - * Contract: the caller should call stop() first, - * - * @param mimeType the mime type we want to show - * @param serviceName allows to enforce a particular service to be chosen, - * @see KonqFactory. - */ - bool changeViewMode( const QString &mimeType, - const QString &serviceName = QString(), - bool forceAutoEmbed = false ); + /** + * Change the part inside this view if necessary. + * Contract: the caller should call stop() first. + * + * @param mimeType the mime type we want to show + * @param serviceName allows to enforce a particular service to be chosen, + * @see KonqFactory. + * @param forceAutoEmbed + */ + bool changePart(const QString &mimeType, + const QString &serviceName = QString(), + bool forceAutoEmbed = false); + /** + * Ensures that this view's part supports the given @p mimeType, + * otherwise calls changePart. + */ + bool ensureViewSupports(const QString& mimeType, + bool forceAutoEmbed); + /** * Call this to prevent next openUrl() call from changing history lists * Used when the same URL is reloaded (for instance with another view mode) @@ -353,7 +361,7 @@ Q_SIGNALS: /** - * Signal the main window that the embedded part changed (e.g. because of changeViewMode) + * Signal the main window that the embedded part changed (e.g. because of changePart) */ void sigPartChanged( KonqView *childView, KParts::ReadOnlyPart *oldPart, KParts::ReadOnlyPart *newPart ); --- trunk/KDE/kdebase/apps/konqueror/src/tests/CMakeLists.txt #788538:788539 @@ -25,3 +25,8 @@ kde4_add_unit_test(konqhtmltest konqhtmltest.cpp) target_link_libraries(konqhtmltest kdeinit_konqueror ${QT_QTTEST_LIBRARY}) + +########### konqviewtest ############### + +kde4_add_unit_test(konqviewtest konqviewtest.cpp) +target_link_libraries(konqviewtest kdeinit_konqueror ${QT_QTTEST_LIBRARY}) --- trunk/KDE/kdebase/apps/konqueror/src/tests/konqhtmltest.cpp #788538:788539 @@ -28,8 +28,6 @@ class KonqHtmlTest : public QObject { Q_OBJECT -public: - private Q_SLOTS: void loadSimpleHtml() {