Bug 153033 - jumping to #anchor on pages with static elements triggers invalid/stuck/non-scrolling repaint state
Summary: jumping to #anchor on pages with static elements triggers invalid/stuck/non-s...
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: 4.0
Platform: Compiled Sources Linux
: NOR grave
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-28 07:51 UTC by Germain Garand
Modified: 2008-04-20 17:46 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Example patch (488 bytes, patch)
2007-11-29 13:38 UTC, Allan Sandfeld
Details
New patch (1.64 KB, patch)
2007-12-19 14:02 UTC, Allan Sandfeld
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Germain Garand 2007-11-28 07:51:50 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources

play a bit with a page with anchor and static position to reproduce that one. e.g:
http://www.w3.org/TR/html401/struct/lists.html#h-10.2

Go back, forward, reload, place cursor in address bar and press enter...

seem to be related to the way static background pages is implemented (it needs view()->widget() positioned at 0,0 all the time).

Jump probably happens at such an early time that it does not trigger  QScrollArea::scrollContentsBy and the widget ends up stuck at some offset.
Comment 1 Allan Sandfeld 2007-11-28 13:32:05 UTC
I can get the page to only render the fixed background, but it is still scrollable. 

I've seen some other pages that ended up really wrong, but I can't trigger it here.
Comment 2 Germain Garand 2007-11-28 16:42:06 UTC
if I click the link, it works OK. But if I paste the url in a new tab's address bar and press enter, the invalid state is triggered.
Comment 3 Germain Garand 2007-11-28 16:45:06 UTC
(blank page, displaying artifacts when resizing the view, except if you scroll all the way up so that the widget is indeed at 0,0)
Comment 4 Allan Sandfeld 2007-11-29 10:42:26 UTC
Okay, I can trigger it now. It is pretty crazy. When resizing you can end up with the scrollbar rendered streched over the screen. To me it seems as if the page isn't repainted at the new size, and the old buffered image is reused at a wrong size creating streching artifacts.

This has to be a wrong interaction with Qt, either a bug in Qt, or a wrongly set flag.
Comment 5 Allan Sandfeld 2007-11-29 13:37:03 UTC
I've traced it. The bug is in the QScrollArea. 

It has an eventFilter that catches resizeEvents, and performs its own widget->move() on them. This bypases our scrolling system.

I think the solution probably requires inheriting from QAbstractScrollArea instead, or finding another way to handle fixed content.
Comment 6 Allan Sandfeld 2007-11-29 13:38:26 UTC
Created attachment 22249 [details]
Example patch

This patch demonstrates what is wrong. It tries to undo the damage on every
resizeEvent. It is however not an effective solution.
Comment 7 Germain Garand 2007-11-29 14:49:53 UTC
> I've traced it. The bug is in the QScrollArea. 
>  
>  It has an eventFilter that catches resizeEvents, and performs its own
>  widget->move() on them.

ah right, nice catch, I can see it

> I think the solution probably requires inheriting from QAbstractScrollArea
> instead, or finding another way to handle fixed content.

well if it's in an eventFilter, we can either catch the event in our own filter and reimplement the scrollbar adjustment, or we can undo the bogus move as you suggest. The latter looks like a workable solution.

It's not like there is any performance problem with moving the widget within a resize event.

> This patch demonstrates what is wrong. It tries to undo the damage on every 
> resizeEvent. It is however not an effective solution. 

'not effective' as in not fully working? I think it just misses a similar check in scrollContentsBy() as there seem to be also a race condition sometimes preventing the initial move(0,0) from happening.

Comment 8 Allan Sandfeld 2007-11-29 16:16:57 UTC
There is indeed a race somewhere during loading and gotoAnchor. I know of a semi-related bug, that is unrelated to fixed content. When using a site like slashdot.org, and clicking on links to comments, Konqueror will often load the link in an odd state. Basically if the anchor is one the first visible page, the viewport is sometimes moved instead of the content, this way the viewport doesn't fill the screen, so the lower part of the screen is black, but the content scrolling correctly at the top. Example: http://slashdot.org/comments.pl?sid=373791&threshold=0&commentsort=0&mode=thread&pid=21514445#21514969

This race condition I believe might be the same that inhibits my patch. I would prefer to solve it generally.
Comment 9 Allan Sandfeld 2007-12-02 12:41:29 UTC
SVN commit 743928 by carewolf:

Work-around that QScrollArea hasn't fully abstracted the scrolling mechanism, 
and sometimes scrolls without our intervention.
* Fixes another showshopping bug.
BUG: 153033


 M  +21 -17    khtmlview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=743928
Comment 10 Allan Sandfeld 2007-12-19 13:59:56 UTC
Bug is still open. The symptoms are just reduced
Comment 11 Allan Sandfeld 2007-12-19 14:02:40 UTC
Created attachment 22621 [details]
New patch

Patch fixing even more cases of this bug. I can still trigger misjumps every
now and then though.
Comment 12 Germain Garand 2007-12-20 04:18:40 UTC
do you have some page where it's reproducible? I didn't see such problems since you closed.

the strange thing is your patch basically says scrollContentsBy does not get called. 
I can't see how this can happen as the scrollbar's valueChanged() signal is wired to scrollContentsBy from as soon as QAbstractScrollArea's constructor.

did you try to assert(!signalsBlocked()) from in there?
Comment 13 Allan Sandfeld 2007-12-20 09:53:11 UTC
Try the slashdot link. Generally clicking on any early comment in a thread will result in a half painted page.
Comment 14 Germain Garand 2007-12-20 16:43:55 UTC
mmh, no I can't reproduce there.
Comment 15 Kevin Funk 2008-04-20 16:16:01 UTC
Works fine for me using 4.0.3.
Comment 16 Germain Garand 2008-04-20 17:46:46 UTC
yes, this is fixed now