Bug 5759

Summary: Links to anchors jump to wrong place (horizontally)
Product: WebKit Reporter: Christopher Stevenson <chris>
Component: Layout and RenderingAssignee: Adele Peterson <adele>
Status: RESOLVED FIXED    
Severity: Normal CC: vicki
Priority: P1    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
testcase html page
none
Patch to fix anchor scrolling problem
none
new patch for anchor scrolling adele: review+

Description Christopher Stevenson 2005-11-16 09:58:42 PST
When Webkit jumps horizontally to an anchor it displays the begining of it in the top right and not the top 
left of the browser (despite the code having a work-around which is sound for IE PC).

Test case page at :

http://www.aplacecalledcommon.co.uk/testcase/index.html
Comment 1 Christopher Stevenson 2005-11-16 09:59:52 PST
Created attachment 4698 [details]
testcase html page
Comment 2 mitz 2005-11-16 12:06:19 PST
Confirmed on TOT. I think this is a regression from after bug 4964 was fixed. The testcase for that bug is 
broken as well on TOT.
Comment 3 mitz 2005-11-16 12:10:12 PST
P1 since it's a regression from WebKit-416.11
Comment 4 Vicki Murley 2005-11-16 13:05:23 PST
Adele, looks like <rdar://problem/4318167> REGRESSION: content doesn't scroll far enough to the left 
after clicking links at aplacecalledcommon.co.uk is back!
Comment 5 Vicki Murley 2005-11-16 13:22:45 PST
In Radar as <rdar://problem/4346132>
Comment 6 Adele Peterson 2005-11-16 14:38:19 PST
Created attachment 4704 [details]
Patch to fix anchor scrolling problem

This patch fixes two bugs.  First, in getRect- if the width or the height was
0, then we would set both the width and height to -1.  This is clearly wrong. 
Second, in scrollRectToVisible - if we were trying to do the minimum amount of
scrolling, and a rect was off-screen to the right, we scroll to line up the
right edge of the rect, to the right edge of the window.  This doesn't work
right if the node's rect is bigger than the window.  So now we check for that,
and align to the left side in that case.
Comment 7 David Harrison 2005-11-16 14:57:26 PST
Looks good except don't you still want to return an empty rect rather than one with a negative width or 
height?

if ( xEnd < xPos || yEnd < yPos )
    return QRect( QPoint( xPos, yPos ), QSize() );
Comment 8 Adele Peterson 2005-11-16 15:00:51 PST
excellent point Dave!  I'll attach a new patch
Comment 9 Adele Peterson 2005-11-16 15:02:16 PST
well...if the width is negative, don't we want to only set that to 0?  and not the height also in case the 
height is positive?
Comment 10 Adele Peterson 2005-11-16 15:16:54 PST
Created attachment 4705 [details]
new patch for anchor scrolling
Comment 11 Adele Peterson 2005-11-16 16:14:54 PST
Comment on attachment 4705 [details]
new patch for anchor scrolling

Dave Harrison reviewed this