Bug 14867 - Fails to scroll to top of page when loading URL w/ empty reference fragment
Summary: Fails to scroll to top of page when loading URL w/ empty reference fragment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Normal
Assignee: Yuzhu Shen
URL: http://wiki.gamer.com.tw/wiki.php?n=P...
Keywords:
Depends on:
Blocks:
 
Reported: 2007-08-02 19:00 PDT by Darin Fisher (:fishd, Google)
Modified: 2008-03-11 23:19 PDT (History)
3 users (show)

See Also:


Attachments
reduced test case (235 bytes, text/html)
2007-08-02 19:01 PDT, Darin Fisher (:fishd, Google)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2007-08-02 19:00:47 PDT
Fails to scroll to top of page when loading URL w/ empty reference fragment

Observed in the latest Safari3 as well as WebKit r24803.  IE and FF do not have this bug.

If you have a page w/ an anchor like <a href="#">, other browsers scroll to the top of the page, but WebKit does not.

This causes problems on pages like this:
http://wiki.gamer.com.tw/wiki.php?n=PSU%3A%E5%A4%A2%E5%B9%BB%E4%B9%8B%E6%98%9F

If you scroll down to the bottom of the page, there is an arrow button that is intended to take you back to the top of the page.  It is implemented using <a href="#">.

Simple test case coming up...
Comment 1 Darin Fisher (:fishd, Google) 2007-08-02 19:01:25 PDT
Created attachment 15813 [details]
reduced test case
Comment 2 David Kilzer (:ddkilzer) 2007-08-03 05:57:36 PDT
Expected behavior works in Firefox 2.0.0.6.  Does not work in Opera 9.21.

Comment 3 Yuzhu Shen 2008-03-10 22:05:48 PDT
With <a href="#">, WebKit interprets the fragment identifier as a null string.

And then WebKit searches the anchor node in two steps: (in bool FrameLoader::gotoAnchor(const String& name))
1) searches any node whose id attribute is the same as the fragment identifier.
   If the fragment identifier is a null string, it returns directly.

2) searches in a set of <a> elements that have name attributes, using HTMLCollection::namedItem().
   It should be noted that namedItem() first searches for an object with a matching id attribute. If a match is not 
found, the method then searches for an object with a matching name attribute.

   Consider the following node:
   <a name="anyname">...</a>
   Its id attribute is not specified (returned as a null string when querying it). If we are searching with a null 
fragment identifier, it will match this node, no matter what its name attribute is.

As a result, clicking the "Goto Top" link in the page below will bring you to the "youwillgethere" anchor but not the 
top of the page.

=======================================
...
<a href="#">Goto Top</a>
...
...
<a name="youwillgethere">Oops!</a>
...
=======================================

I will upload a patch soon.
Comment 4 Robert Blaut 2008-03-10 23:36:11 PDT
As I tested today Webkit r30939 scrolls to the top. So the reported issue is absent in this build. But there is similar issue reported in bug 17714.
Comment 5 Yuzhu Shen 2008-03-10 23:46:37 PDT
Yes, I tested with the newest version of the code and the problem disappears.

BTW, in case like this, shall I still submit a patch with only the test case and expeced result?
Comment 6 Robert Blaut 2008-03-11 06:26:42 PDT
(In reply to comment #5)
> Yes, I tested with the newest version of the code and the problem disappears.
> 
> BTW, in case like this, shall I still submit a patch with only the test case
> and expeced result?
> 

I think new layout tests are always welcome :)
Comment 7 Yuzhu Shen 2008-03-11 19:50:33 PDT
I have uploaded a patch for bug 17714 and it has been reviewed. This two bugs are related, please refer to my comments in bug 17714 for details. And the test case for bug 17714 covers the situation described in this bug.
Comment 8 Robert Blaut 2008-03-11 23:19:35 PDT
(In reply to comment #7)
> I have uploaded a patch for bug 17714 and it has been reviewed. This two bugs
> are related, please refer to my comments in bug 17714 for details. And the test
> case for bug 17714 covers the situation described in this bug.
> 

So I mark the bug as FIXED since original reported bug is not visible any more and the test case for it is available in related bug 17714.