Bug 140777

Summary: immediate redraw of visited links makes page navigation noticably slower
Product: konqueror Reporter: Leo Savernik <l.savernik>
Component: khtmlAssignee: Martin Koller <kollix>
Status: RESOLVED FIXED    
Severity: normal CC: germain
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:

Description Leo Savernik 2007-01-28 14:38:09 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources

This bug is reported against KDE 3.5.6 (which cannot be selected from the list at the time of this writing).

Since KDE 3.5.6, khtml immediately marks links as visited when another page is loaded.

However, this leads to a considerable slowdown of page loading times (and accompanying bugs like bug 140768) especially noticeable on older hardware.

The problem is an unconditional style recalculation and full repaint *as soon as a new history entry is created*. Both operations need a very long time (compared to incremental painting operations) to complete to such an extent that they become user-noticeable.

A temporary fix is to apply this patch which basically reverts the functionality:

Index: html/html_documentimpl.cpp
===================================================================
--- html/html_documentimpl.cpp  (Revision 627229)
+++ html/html_documentimpl.cpp  (Arbeitskopie)
@@ -222,7 +222,7 @@

 void HTMLDocumentImpl::slotHistoryChanged()
 {
-    if ( !m_render )
+    if ( true || !m_render )
         return;

     recalcStyle( Force );


See also https://bugzilla.mozilla.org/show_bug.cgi?id=78510
on how Mozilla tackled this problem and which different solutions they pointed out.

I suggest reverting r617941 and implementing the 95% case for now (i. e. mark the link and only that link as being visited as soon as it has been activated). This incurs a relayout of the anchor element only which usually boils down to a repaint only (as long as color or text-decoration is changed only).

A proper fix must never do a full relayout+repaint, but issue repaints on the individual anchor elements only.
Comment 1 Leo Savernik 2007-01-28 15:16:24 UTC
Part of Mozilla's solution strategy may actually help khtml's, too:
https://bugzilla.mozilla.org/show_bug.cgi?id=78510#c128
Comment 2 Germain Garand 2007-01-30 15:37:31 UTC
Please apply the partial revert now (with a CCMAIL:kde-packager@kde.org)...
there's no reason to have everybody harmed while waiting for a proper solution.

When I think this was in svn for 4 weeks before release and nobody noticed anything wrong!

Clearly far too few people test branch.
Meaningful development or backport cannot continue with such poor conditions.

Comment 3 Leo Savernik 2007-01-30 18:25:40 UTC
SVN commit 628618 by savernik:

Reverting r617941. This fixes jumping to the top right before loading a
new page and also fixes page loading time increase.

Attention packagers! Please include this patch in new versions of your
khtml-3.5.6 packages. Web surfing experience can be considered broken
without it.

CCMAIL: kde-packager@kde.org
BUG: 140768
CCBUG: 140777    
CCBUG: 24820


 M  +1 -1      html_documentimpl.cpp  


--- branches/KDE/3.5/kdelibs/khtml/html/html_documentimpl.cpp #628617:628618
@@ -222,7 +222,7 @@
 
 void HTMLDocumentImpl::slotHistoryChanged()
 {
-    if ( !m_render )
+    if ( true || !m_render )
         return;
 
     recalcStyle( Force );
Comment 4 Leo Savernik 2007-01-30 18:42:29 UTC
Reverted. How about requiring approval for non-core khtml committers' patches like in KDE 3.4-times? ... Oh, it *was* approved. Well, not sure how to maintain quality without deterring occasional committers from contributing.

But maybe we've reached the time to become convervative about what's committed to the branch.

(Just for the records: The original bug r617941 was about to fix was bug 24820.)