Bug 28954 - hostname and host are mixed up when manipulating anchor elements
Summary: hostname and host are mixed up when manipulating anchor elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 20608 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-09-03 13:47 PDT by Yael
Modified: 2009-09-06 22:25 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.88 KB, patch)
2009-09-03 14:37 PDT, Yael
darin: review-
Details | Formatted Diff | Diff
Patch (6.29 KB, patch)
2009-09-04 09:09 PDT, Yael
darin: review+
Details | Formatted Diff | Diff
Patch (3.51 KB, patch)
2009-09-04 14:07 PDT, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2009-09-03 13:47:14 PDT
From http://dev.w3.org/html5/spec/infrastructure.html#interfaces-for-url-manipulation:

o . host [ = value ]
    Returns the current host and port (if it's not the default port) in the underlying URL.
    Can be set, to change the underlying URL's host and port.
    The host and the port are separated by a colon. The port part, if omitted, will be assumed to be the current scheme's default port.
o . hostname [ = value ]
    Returns the current host in the underlying URL.
    Can be set, to change the underlying URL's host.


In the current implementation of HTMLAnchorElement, they are mixed up.
FireFox and IE are following the spec while WebKit, Safari and Opera are not.
Comment 1 Yael 2009-09-03 14:37:01 PDT
Created attachment 39012 [details]
Patch
Comment 2 Darin Adler 2009-09-03 14:47:56 PDT
Comment on attachment 39012 [details]
Patch

Change looks good. This needs a regression test. You should be able to test it with a test in the "http" part of the LayoutTests directory.
Comment 3 Yael 2009-09-04 09:09:39 PDT
Created attachment 39064 [details]
Patch

Added a test as requested in #2.
Comment 4 Yael 2009-09-04 10:53:55 PDT
Landed in r48063.
Comment 5 Darin Adler 2009-09-04 10:58:23 PDT
What about a port number of 0? This code seems to do the wrong thing in that case. Maybe you could do a separate patch that adds test cases for that case.
Comment 6 Yael 2009-09-04 11:26:32 PDT
(In reply to comment #5)
> What about a port number of 0? This code seems to do the wrong thing in that
> case. Maybe you could do a separate patch that adds test cases for that case.

You are right, when the href includes port 0, my patch does not handle it correctly. I will fix it shortly.
Comment 7 Alexey Proskuryakov 2009-09-04 12:41:47 PDT
*** Bug 20608 has been marked as a duplicate of this bug. ***
Comment 8 Yael 2009-09-04 14:07:24 PDT
Created attachment 39089 [details]
Patch

Added handling for when port is 0, including a new test case.
Comment 9 Yael 2009-09-04 14:08:08 PDT
Due to additional comments, I am reopening this bug.
Comment 10 Darin Adler 2009-09-05 21:12:35 PDT
Comment on attachment 39089 [details]
Patch

> +    if (url.hostEnd() == url.pathStart())

I think it's slightly unclear to code it this way. I'd prefer to have a hasPort() function on KURL. In fact, at one point these hostEnd() and pathStart() functions didn't exist, and I'm not sure we'll want to keep them in the long run.

But I guess this is OK as-is for now. I won't insist on changing KURL.h.

r=me
Comment 11 Eric Seidel (no email) 2009-09-06 22:25:08 PDT
Comment on attachment 39089 [details]
Patch

Clearing flags on attachment: 39089

Committed r48107: <http://trac.webkit.org/changeset/48107>
Comment 12 Eric Seidel (no email) 2009-09-06 22:25:13 PDT
All reviewed patches have been landed.  Closing bug.