Bug 140777 - immediate redraw of visited links makes page navigation noticably slower
Summary: immediate redraw of visited links makes page navigation noticably slower
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Unclassified
Component: khtml (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Martin Koller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-28 14:38 UTC by Leo Savernik
Modified: 2012-01-29 14:47 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.)