Bug 16214 – rename KURL to URL

• Component: Platform
• Status: RESOLVED FIXED

[Darin Adler]

Going to do it.

[Darin Adler]

Created attachment 17616 [details]
patch for scrubbing URL out of the tree (step 1)

[Darin Adler]

Created attachment 17828 [details]
newer URL -> url patch

[Sam Weinig]

Created attachment 17837 [details]
newer URL -> url patch (now with Windows support and changelogs)

[Darin Adler]

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.

[Sam Weinig]

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.

[Geoffrey Garen]

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.

[Darin Adler]

(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.

[Adam Barth]

> > 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).

[Darin Adler]

Created attachment 212788 [details]
Patch

[Build Bot]

Comment on attachment 212788 [details]
Patch

Attachment 212788 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/2655100

[Andreas Kling]

Created attachment 212806 [details]
Patch

[Andreas Kling]

Created attachment 212807 [details]
Patch

Oops, didn't get Tools/

[Andreas Kling]

Created attachment 212808 [details]
Patch

[Early Warning System Bot]

Comment on attachment 212808 [details]
Patch

Attachment 212808 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/2660145

[Early Warning System Bot]

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

[Darin Adler]

Created attachment 212809 [details]
Patch

[Darin Adler]

Created attachment 212811 [details]
Patch

[Andreas Kling]

Comment on attachment 212811 [details]
Patch

r=me

[Darin Adler]

http://trac.webkit.org/changeset/156550

[Timothy Hatcher]

This broke the Windows builds.

http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29
http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29

[Timothy Hatcher]

Attempted a Windows fix in https://trac.webkit.org/r156552.

[Timothy Hatcher]

Windows is building again.

[Alexey Proskuryakov]

Updated bindings test results in r156580.