Bug 34550

Summary: REGRESSION (r51644): WebCore/manual-tests/linkjump-1.html fails
Product: WebKit Reporter: Brady Eidson <beidson>
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal Keywords: InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
1 line fix, updated comment, and a veritable cornucopia of layout tests ap: review+, beidson: commit-queue-

Brady Eidson
Reported 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.
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-
Brady Eidson
Comment 1 2010-02-03 18:05:21 PST
Brady Eidson
Comment 2 2010-02-03 18:08:04 PST
Created attachment 48088 [details] 1 line fix, updated comment, and a veritable cornucopia of layout tests
Alexey Proskuryakov
Comment 3 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?
Brady Eidson
Comment 4 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.
Brady Eidson
Comment 5 2010-02-03 19:28:56 PST
Alexey Proskuryakov
Comment 6 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.
Brady Eidson
Comment 7 2010-02-03 19:44:42 PST
rs=you on removing it and logging that commit here? :)
Note You need to log in before you can comment on or make changes to this bug.