Bug 5759 – Links to anchors jump to wrong place (horizontally)

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Christopher Stevenson]

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

[Christopher Stevenson]

Created attachment 4698 [details]
testcase html page

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.

P1 since it's a regression from WebKit-416.11

[Vicki Murley]

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!

[Vicki Murley]

In Radar as <rdar://problem/4346132>

[Adele Peterson]

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.

[David Harrison]

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() );

[Adele Peterson]

excellent point Dave!  I'll attach a new patch

[Adele Peterson]

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?

[Adele Peterson]

Created attachment 4705 [details]
new patch for anchor scrolling

[Adele Peterson]

Comment on attachment 4705 [details]
new patch for anchor scrolling

Dave Harrison reviewed this