Bug 55415

Summary: KURL should expose a referrer property.
Product: WebKit Reporter: David Levin <levin>
Component: PlatformAssignee: David Levin <levin>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, dglazkov, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 24683    
Attachments:
Description Flags
Patch
levin: review-
Patch darin: review+

Description David Levin 2011-02-28 14:58:30 PST
This is done to allow re-use of this algorithm which is currently only in FrameLoader::setOutgoingReferrer.
Comment 1 David Levin 2011-02-28 15:00:00 PST
Created attachment 84128 [details]
Patch
Comment 2 Darin Adler 2011-02-28 15:11:04 PST
Comment on attachment 84128 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84128&action=review

> Source/WebCore/platform/KURL.cpp:1772
> +KURL KURL::referrer() const

I don’t think referrer is the right name for this. A function of this name makes it seem like URLs have referrers, and this returns the referrer of a URL. We could also consider having this return a string rather than a URL.
Comment 3 David Levin 2011-02-28 15:15:04 PST
Comment on attachment 84128 [details]
Patch

r- based on Darin's comment.
Comment 4 WebKit Review Bot 2011-02-28 15:21:46 PST
Attachment 84128 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8075404
Comment 5 David Levin 2011-02-28 17:01:59 PST
Created attachment 84156 [details]
Patch
Comment 6 Darin Adler 2011-02-28 17:10:43 PST
Comment on attachment 84156 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84156&action=review

> Source/WebCore/platform/KURL.cpp:1772
> +String KURL::toReferrer() const

Certainly better than just referrer. Maybe strippedForUseAsReferrer would be even better even though it’s wordy.
Comment 7 David Levin 2011-02-28 17:19:36 PST
(In reply to comment #6)
> (From update of attachment 84156 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84156&action=review
> 
> > Source/WebCore/platform/KURL.cpp:1772
> > +String KURL::toReferrer() const
> 
> Certainly better than just referrer. Maybe strippedForUseAsReferrer would be even better even though it’s wordy.

Best of two: What about toStrippedReferrer vs strippedForUseAsReferrer?

I'll go with whatever you think it better.
Comment 8 Darin Adler 2011-02-28 17:31:47 PST
(In reply to comment #7)
> Best of two: What about toStrippedReferrer vs strippedForUseAsReferrer?
> 
> I'll go with whatever you think it better.

Despite using it myself for things like typecasts, I am not big on the “to” prefix, so I think I prefer the “stripped” version I suggested. I wish I liked yours better because I don’t want to be accused of bias ;-)

Another way to go is to have it mutate a URL and call it stripForUseAsReferrer.
Comment 9 David Levin 2011-02-28 17:41:00 PST
(In reply to comment #8)
> Despite using it myself for things like typecasts, I am not big on the “to” prefix, 
ha, I mentally (create, generate) tried a few different names and settled on "to" because it seemed like common WebKit nomenclature.

> I wish I liked yours better because I don’t want to be accused of bias ;-)
Bias is fine. Mine was simply an attempt to wordsmith a shorter name. (I'm not sure I liked it -- it felt like it may cut out too much.)

> Another way to go is to have it mutate a URL and call it stripForUseAsReferrer.
I like not mutating KURL, and this method would simply result in more (boilerplate) code at the calling site anyway.

I'll go with: strippedForUseAsReferrer

Thanks!
Comment 10 David Levin 2011-02-28 18:11:07 PST
Commited as http://trac.webkit.org/changeset/79956