Bug 34550 - REGRESSION (r51644): WebCore/manual-tests/linkjump-1.html fails
Summary: REGRESSION (r51644): WebCore/manual-tests/linkjump-1.html fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-02-03 17:59 PST by Brady Eidson
Modified: 2010-02-03 19:44 PST (History)
0 users

See Also:


Attachments
1 line fix, updated comment, and a veritable cornucopia of layout tests (14.00 KB, patch)
2010-02-03 18:08 PST, Brady Eidson
ap: review+
beidson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2010-02-03 17:59:59 PST
http://trac.webkit.org/changeset/51644 caused manual-tests/linkjump to fail.

Previously, fragment scrolls didn't actually update a Document's URL.  After r51644, they did, causing improper relative URL calculation when clicking on anchors that only contain an (empty) fragment identifier.

Turns out that when computing a new URL with a base and relative URL, KURL didn't correctly handle the case of an empty fragment ID.

A comment in the relevant code correctly called out the fact that the RFC notes that an empty reference relative to an absolute URL means the new URL refers to the same document, but left out the fact that it refers to the same document *with any existing fragment identifier removed*

One line fix + layout tests are coming up.
Comment 1 Brady Eidson 2010-02-03 18:05:21 PST
<rdar://problem/7595694>
Comment 2 Brady Eidson 2010-02-03 18:08:04 PST
Created attachment 48088 [details]
1 line fix, updated comment, and a veritable cornucopia of layout tests
Comment 3 Alexey Proskuryakov 2010-02-03 19:15:11 PST
Comment on attachment 48088 [details]
1 line fix, updated comment, and a veritable cornucopia of layout tests 

r=me

Do these tests cover cases that used to be covered by manual tests? If not, can the latter be converted to automated ones, as well?
Comment 4 Brady Eidson 2010-02-03 19:26:31 PST
These tests definitely (largely) overlap with the original manual-tests/linkjump-1.html test using "EventSender Technology™" unavailable in the dark ages, and I suspect they cover the original manual test.

If you think it should be removed, I'd like to do that in a separate patch.
Comment 5 Brady Eidson 2010-02-03 19:28:56 PST
http://trac.webkit.org/changeset/54321
Comment 6 Alexey Proskuryakov 2010-02-03 19:34:10 PST
>If you think it should be removed, I'd like to do that in a separate patch.

It's nice to reduce the number of manual tests when possible.
Comment 7 Brady Eidson 2010-02-03 19:44:42 PST
rs=you on removing it and logging that commit here?  :)