Bug 16214 - rename KURL to URL
Summary: rename KURL to URL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Trivial
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks: 37641
  Show dependency treegraph
 
Reported: 2007-11-30 17:25 PST by Darin Adler
Modified: 2013-09-27 15:14 PDT (History)
6 users (show)

See Also:


Attachments
patch for scrubbing URL out of the tree (step 1) (129.33 KB, patch)
2007-11-30 17:25 PST, Darin Adler
no flags Details | Formatted Diff | Diff
newer URL -> url patch (129.44 KB, patch)
2007-12-10 14:44 PST, Darin Adler
no flags Details | Formatted Diff | Diff
newer URL -> url patch (now with Windows support and changelogs) (157.78 KB, patch)
2007-12-10 21:47 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (1.52 MB, patch)
2013-09-26 22:42 PDT, Darin Adler
kling: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch (1.36 MB, patch)
2013-09-27 07:25 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch (1.38 MB, patch)
2013-09-27 07:30 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch (1.38 MB, patch)
2013-09-27 07:40 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch (1.52 MB, patch)
2013-09-27 07:55 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (1.52 MB, patch)
2013-09-27 08:20 PDT, Darin Adler
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2007-11-30 17:25:22 PST
Going to do it.
Comment 1 Darin Adler 2007-11-30 17:25:51 PST
Created attachment 17616 [details]
patch for scrubbing URL out of the tree (step 1)
Comment 2 Darin Adler 2007-12-10 14:44:02 PST
Created attachment 17828 [details]
newer URL -> url patch
Comment 3 Sam Weinig 2007-12-10 21:47:39 PST
Created attachment 17837 [details]
newer URL -> url patch (now with Windows support and changelogs)
Comment 4 Darin Adler 2007-12-11 16:00:06 PST
Comment on attachment 17837 [details]
newer URL -> url patch (now with Windows support and changelogs)

+        Scrub URL out of the tree in preparation of ranameing KURL to URL.

Renaming is spelled wrong here. Also "in preparation of" is not very good English.

+        * WebCore.base.exp:
+        * bindings/js/kjs_events.cpp:
+        (WebCore::JSLazyEventListener::parseCode):

I don't think it is all that helpful to list all the files and functions when the only change is renaming. It would be neat to list only files that have interesting changes; something other than just renaming. Or we could just omit the file list entirely. Or leave it as is, not sure.

1637          DeprecatedString dstUrl = activeFrame->loader()->completeURL(DeprecatedString(args[0]->toString(exec))).url();
 1637         String dstUrl = activeFrame->loader()->completeURL(args[0]->toString(exec)).deprecatedString();

This is strange. It's changed to call deprecatedString(), but the type of the variable was changed to String!

48      return m_url.url().isNull();
 48     return m_url.deprecatedString().isNull();

We should probably add a KURL.isNull() for this. Not sure why I didn't.

What about platforms other than Mac and Windows.

r=me if you fix the things I mentioned above.
Comment 5 Sam Weinig 2007-12-11 23:34:07 PST
Comment on attachment 17837 [details]
newer URL -> url patch (now with Windows support and changelogs)

First patch landed in r28639.  Removing the review flag to continue using this bug.
Comment 6 Geoffrey Garen 2007-12-11 23:36:01 PST
I thought we wanted to rename KURL to DeprecatedURL or ParsedURL, not URL, right? 

URL gives the incorrect impression that this is the class that you should use when passing around URLs when, in reality, a String is just fine in many cases. Also, KURL is built on top of DeprecatedString.
Comment 7 Darin Adler 2007-12-12 07:46:46 PST
(In reply to comment #6)
> I thought we wanted to rename KURL to DeprecatedURL or ParsedURL, not URL,
> right? 

I discussed this at length with Maciej and a few others. I can't remember all the arguments back and forth but we did settle on URL.

> URL gives the incorrect impression that this is the class that you should use
> when passing around URLs when, in reality, a String is just fine in many cases.

That's true. But URL provides some benefits. I really wish I could remember the details of the discussion. I thought you were involved.

> Also, KURL is built on top of DeprecatedString.

That's temporary. We will be changing that.
Comment 8 Adam Barth 2011-12-21 23:53:49 PST
> > URL gives the incorrect impression that this is the class that you should use
> > when passing around URLs when, in reality, a String is just fine in many cases.
> 
> That's true. But URL provides some benefits. I really wish I could remember the details of the discussion. I thought you were involved.

In the past, we've talked about having a URLString class for passing around Strings that are URLs (i.e., that have already been validated as URLs).
Comment 9 Darin Adler 2013-09-26 22:42:48 PDT
Created attachment 212788 [details]
Patch
Comment 10 Build Bot 2013-09-27 03:02:23 PDT
Comment on attachment 212788 [details]
Patch

Attachment 212788 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/2655100
Comment 11 Andreas Kling 2013-09-27 07:25:04 PDT
Created attachment 212806 [details]
Patch
Comment 12 Andreas Kling 2013-09-27 07:30:12 PDT
Created attachment 212807 [details]
Patch

Oops, didn't get Tools/
Comment 13 Andreas Kling 2013-09-27 07:40:48 PDT
Created attachment 212808 [details]
Patch
Comment 14 Early Warning System Bot 2013-09-27 07:51:33 PDT
Comment on attachment 212808 [details]
Patch

Attachment 212808 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/2660145
Comment 15 Early Warning System Bot 2013-09-27 07:52:15 PDT
Comment on attachment 212808 [details]
Patch

Attachment 212808 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/2668049
Comment 16 Darin Adler 2013-09-27 07:55:18 PDT
Created attachment 212809 [details]
Patch
Comment 17 Darin Adler 2013-09-27 08:20:20 PDT
Created attachment 212811 [details]
Patch
Comment 18 Andreas Kling 2013-09-27 08:23:25 PDT
Comment on attachment 212811 [details]
Patch

r=me
Comment 19 Darin Adler 2013-09-27 09:45:13 PDT
http://trac.webkit.org/changeset/156550
Comment 21 Timothy Hatcher 2013-09-27 10:24:49 PDT
Attempted a Windows fix in https://trac.webkit.org/r156552.
Comment 22 Timothy Hatcher 2013-09-27 10:27:29 PDT
Windows is building again.
Comment 23 Alexey Proskuryakov 2013-09-27 15:14:11 PDT
Updated bindings test results in r156580.