[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    KHelpCenter navigation and sidebar patches
From:       David A Benjamin <davidben () MIT ! EDU>
Date:       2010-04-17 21:47:48
Message-ID: alpine.DEB.1.10.1004171732310.29237 () dr-wily ! mit ! edu
[Download RAW message or body]

Hi all,

I recently attached patches for bugs #179427 and #211545 [1] on 
bugs.kde.org, but someone suggested[2] that I email them instead and CC 
dfaure?

The first patch fixes the next/previous page functions in khelpcenter, 
which is the bug in #211545. The second uses the BrowserExtension to open 
URLs so that various signals are emitted and history is maintained (is 
this correct use of the API?).

The third moves the TOC building code into the NavigatorItem so to be 
called on every setOpen. This fixes #179427.


David Benjamin


[1] Links for the lazy:
     https://bugs.kde.org/show_bug.cgi?id=211545
     https://bugs.kde.org/show_bug.cgi?id=179427

[2] https://bugs.kde.org/show_bug.cgi?id=211545#c12
["khelpcenter-Fix-next-and-previous-page-functions-in-khelpcenter.patch" (TEXT/x-diff)]

From 54c9a8467b6c607729c54c18c584cd134f0f6d54 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@mit.edu>
Date: Wed, 7 Apr 2010 17:05:11 -0400
Subject: [PATCH] Fix next and previous page functions in khelpcenter

This fixes #211545. This removes a ton of ad-hoc code in favor of
existing working library code.

- Instead of counting anchor tags, we use the appropriate <link> tags
  and try to find rel="next"/rel="prev"

- We cast to HTMLLinkElement early to take advantage of convenience
  methods, add self documentation (and possibly performance?).

- We use KUrl's constructor to resolve relative and absolute URLs
  instead of an ad-hoc solution.
---
 runtime/khelpcenter/view.cpp |   64 +++++++++++++++--------------------------
 runtime/khelpcenter/view.h   |    3 +-
 2 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/runtime/khelpcenter/view.cpp b/runtime/khelpcenter/view.cpp
index cf147c2..8404cd5 100644
--- a/runtime/khelpcenter/view.cpp
+++ b/runtime/khelpcenter/view.cpp
@@ -4,6 +4,7 @@
 #include "history.h"
 
 #include <dom/html_document.h>
+#include <dom/html_head.h>
 #include <dom/html_misc.h>
 #include <kaction.h>
 #include <kactioncollection.h>
@@ -220,17 +221,24 @@ void View::slotCopyLink()
   QApplication::clipboard()->setText(mCopyURL);
 }
 
+static DOM::HTMLLinkElement findLink(const DOM::NodeList& links, const char *rel)
+{ 
+  for (unsigned i = 0; i <= links.length(); i++) {
+    DOM::HTMLLinkElement link(links.item(i));
+    if (link.isNull())
+      continue;
+
+    if (link.rel() == rel)
+      return link;
+  }
+  return DOM::HTMLLinkElement();
+}
+
 bool View::prevPage(bool checkOnly)
 {
-  const DOM::HTMLCollection links = htmlDocument().links();
+  const DOM::NodeList links = document().getElementsByTagName("link");
 
-  KUrl prevURL;
-
-  // The first link on a page (top-left corner) would be the Prev link.
-  if ( !baseURL().path().endsWith( QLatin1String("/index.html") ) )
-    prevURL = urlFromLinkNode( links.item( 0 ) );
-  else
-    return false;
+  KUrl prevURL = urlFromLinkNode( findLink(links, "prev") );
 
   if (!prevURL.isValid())
     return false;
@@ -242,27 +250,13 @@ bool View::prevPage(bool checkOnly)
 
 bool View::nextPage(bool checkOnly)
 {
-  const DOM::HTMLCollection links = htmlDocument().links();
+  const DOM::NodeList links = document().getElementsByTagName("link");
 
-  KUrl nextURL;
-
-  // If we're on the first page, the "Next" link is the second to the last link
-  if ( baseURL().path().endsWith( QLatin1String("/index.html") ) )
-    nextURL = urlFromLinkNode( links.item( links.length() - 2 ) );
-  else
-    nextURL = urlFromLinkNode( links.item( links.length() - 4 ) );
+  KUrl nextURL = urlFromLinkNode( findLink(links, "next") );
 
   if (!nextURL.isValid())
     return false;
 
-  // If we get a mail link instead of a http URL, or the next link points
-  // to an index.html page (a index.html page is always the first page
-  // there can't be a Next link pointing to it!) there's probably nowhere
-  // to go. Next link at all.
-  if ( nextURL.protocol() == "mailto" ||
-       nextURL.path().endsWith( QLatin1String("/index.html") ) )
-    return false;
-
   if (!checkOnly)
     openUrl( nextURL );
   return true;
@@ -295,26 +289,16 @@ bool View::eventFilter( QObject *o, QEvent *e )
   return KHTMLPart::eventFilter( o, e );
 }
 
-KUrl View::urlFromLinkNode( const DOM::Node &n ) const
+KUrl View::urlFromLinkNode( const DOM::HTMLLinkElement &link ) const
 {
-  if ( n.isNull() || n.nodeType() != DOM::Node::ELEMENT_NODE )
+  if ( link.isNull() )
     return KUrl();
 
-  DOM::Element elem = static_cast<DOM::Element>( n );
-
-  KUrl href ( elem.getAttribute( "href" ).string() );
-  if ( !href.protocol().isNull() )
-    return href;
-
-  QString path = baseURL().path();
-  path.truncate( path.lastIndexOf( '/' ) + 1 );
-  path += href.url();
-
-  KUrl url = baseURL();
-  url.setRef( QString() );
-  url.setEncodedPathAndQuery( path );
+  DOM::DOMString domHref = link.href();
+  if (domHref.isNull())
+    return KUrl();
 
-  return url;
+  return KUrl(baseURL(), domHref.string());
 }
 
 void View::slotReload( const KUrl &url )
diff --git a/runtime/khelpcenter/view.h b/runtime/khelpcenter/view.h
index 9c89f70..b2c370d 100644
--- a/runtime/khelpcenter/view.h
+++ b/runtime/khelpcenter/view.h
@@ -12,6 +12,7 @@ class KActionCollection;
 
 namespace DOM {
   class Node;
+  class HTMLLinkElement;
 }
 
 namespace KHC {
@@ -72,7 +73,7 @@ class View : public KHTMLPart
     void showMenu( const QString& url, const QPoint& pos);
 
   private:
-    KUrl urlFromLinkNode( const DOM::Node &n ) const;
+    KUrl urlFromLinkNode( const DOM::HTMLLinkElement &link ) const;
  
     int mState;
     QString mTitle;
-- 
1.7.0.rc0.10.gbe06.dirty


["khelpcenter-Use-the-BrowserExtension-to-navigate-between-pages.patch" (TEXT/x-diff)]

From f8f4d2f56beeac696a9b28f7d6667852e50765a2 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@mit.edu>
Date: Mon, 12 Apr 2010 11:53:51 -0400
Subject: [PATCH] Use the BrowserExtension to navigate between pages

This emits the necessary signals for history, sidebar, etc.
---
 runtime/khelpcenter/view.cpp |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/runtime/khelpcenter/view.cpp b/runtime/khelpcenter/view.cpp
index 8404cd5..f90f6d3 100644
--- a/runtime/khelpcenter/view.cpp
+++ b/runtime/khelpcenter/view.cpp
@@ -244,7 +244,7 @@ bool View::prevPage(bool checkOnly)
     return false;
 
   if (!checkOnly)
-    openUrl( prevURL );
+    emit browserExtension()->openUrlRequest(prevURL);
   return true;
 }
 
@@ -258,7 +258,7 @@ bool View::nextPage(bool checkOnly)
     return false;
 
   if (!checkOnly)
-    openUrl( nextURL );
+    emit browserExtension()->openUrlRequest(nextURL);
   return true;
 }
 
-- 
1.7.0.rc0.10.gbe06.dirty


["khelpcenter-Schedule-TOC-builds-as-part-of-a-setOpen.patch" (TEXT/x-diff)]

From d8b848b66c63cea4afe73469452af3675f5cc5fd Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@mit.edu>
Date: Mon, 12 Apr 2010 15:37:13 -0400
Subject: [PATCH] Schedule TOC builds as part of a setOpen

This should fix items not expanding when selected via the arrow. They
also sometimes fail to expand when navigating to the page.
---
 runtime/khelpcenter/navigator.cpp     |   25 +------------------------
 runtime/khelpcenter/navigatoritem.cpp |   23 +++++++++++++++++++++--
 runtime/khelpcenter/navigatoritem.h   |    3 +--
 runtime/khelpcenter/toc.cpp           |    2 --
 4 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/runtime/khelpcenter/navigator.cpp b/runtime/khelpcenter/navigator.cpp
index 1eb2c69..2c67be9 100644
--- a/runtime/khelpcenter/navigator.cpp
+++ b/runtime/khelpcenter/navigator.cpp
@@ -379,8 +379,7 @@ void Navigator::slotItemSelected( Q3ListViewItem *currentItem )
 
   NavigatorItem *item = static_cast<NavigatorItem *>( currentItem );
 
-  kDebug(1400) << "Navigator::slotItemSelected(): " << item->entry()->name()
-                << endl;
+  kDebug(1400) << item->entry()->name() << endl;
 
   if ( item->childCount() > 0 || item->isExpandable() )
     item->setOpen( !item->isOpen() );
@@ -393,28 +392,6 @@ void Navigator::slotItemSelected( Q3ListViewItem *currentItem )
       History::self().createEntry();
       showOverview( item, url );
   } else {
-    if ( url.protocol() == "help" ) {
-      kDebug( 1400 ) << "slotItemSelected(): Got help URL " << url.url()
-                      << endl;
-      if ( !item->toc() ) {
-        TOC *tocTree = item->createTOC();
-        kDebug( 1400 ) << "slotItemSelected(): Trying to build TOC for "
-                        << item->entry()->name() << endl;
-        tocTree->setApplication( url.directory() );
-        QString doc = View::langLookup( url.path() );
-        // Enforce the original .docbook version, in case langLookup returns a
-        // cached version
-        if ( !doc.isNull() ) {
-          int pos = doc.indexOf( ".html" );
-          if ( pos >= 0 ) {
-            doc.replace( pos, 5, ".docbook" );
-          }
-          kDebug( 1400 ) << "slotItemSelected(): doc = " << doc;
-
-          tocTree->build( doc );
-        }
-      }
-    }
     emit itemSelected( url.url() );
   }
 
diff --git a/runtime/khelpcenter/navigatoritem.cpp b/runtime/khelpcenter/navigatoritem.cpp
index f4aa657..80401bd 100644
--- a/runtime/khelpcenter/navigatoritem.cpp
+++ b/runtime/khelpcenter/navigatoritem.cpp
@@ -22,6 +22,7 @@
 
 #include "toc.h"
 #include "docentry.h"
+#include "view.h"
 
 #include <kdebug.h>
 #include <kiconloader.h>
@@ -86,14 +87,32 @@ void NavigatorItem::updateItem()
   setPixmap( 0, SmallIcon( entry()->icon() ) );
 }
 
-TOC *NavigatorItem::createTOC()
+void NavigatorItem::scheduleTOCBuild()
 {
+  KUrl url ( entry()->url() );
+  if ( !mToc && url.protocol() == "help") {
     mToc = new TOC( this );
-    return mToc;
+
+    kDebug( 1400 ) << "Trying to build TOC for " << entry()->name() << endl;
+    mToc->setApplication( url.directory() );
+    QString doc = View::langLookup( url.path() );
+    // Enforce the original .docbook version, in case langLookup returns a
+    // cached version
+    if ( !doc.isNull() ) {
+      int pos = doc.indexOf( ".html" );
+      if ( pos >= 0 ) {
+        doc.replace( pos, 5, ".docbook" );
+      }
+      kDebug( 1400 ) << "doc = " << doc;
+
+      mToc->build( doc );
+    }
+  }
 }
 
 void NavigatorItem::setOpen( bool open )
 {
+  scheduleTOCBuild();
   Q3ListViewItem::setOpen( open );
 
 // TODO: was contents2 -> needs to be changed to help-contents-alternate or similar
diff --git a/runtime/khelpcenter/navigatoritem.h b/runtime/khelpcenter/navigatoritem.h
index 201e583..ba295ac 100644
--- a/runtime/khelpcenter/navigatoritem.h
+++ b/runtime/khelpcenter/navigatoritem.h
@@ -47,13 +47,12 @@ class NavigatorItem : public Q3ListViewItem
     void updateItem();
 
     TOC *toc() const { return mToc; }
-
-    TOC *createTOC();
   
     void setOpen( bool open );
 
   private:
     void init( DocEntry * );
+    void scheduleTOCBuild();
     
     TOC *mToc;
 
diff --git a/runtime/khelpcenter/toc.cpp b/runtime/khelpcenter/toc.cpp
index eb886bd..9e0bef1 100644
--- a/runtime/khelpcenter/toc.cpp
+++ b/runtime/khelpcenter/toc.cpp
@@ -250,8 +250,6 @@ void TOC::fillTree()
             sectItem = new TOCSectionItem( this, chapItem, sectItem, sectTitle, sectRef );
         }
     }
-
-  m_parentItem->setOpen( true );
 }
 
 QDomElement TOC::childElement( const QDomElement &element, const QString &name )
-- 
1.7.0.rc0.10.gbe06.dirty



>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic