Bug 55415 - KURL should expose a referrer property.
Summary: KURL should expose a referrer property.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks: 24683
  Show dependency treegraph
 
Reported: 2011-02-28 14:58 PST by David Levin
Modified: 2011-02-28 18:11 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.86 KB, patch)
2011-02-28 15:00 PST, David Levin
levin: review-
Details | Formatted Diff | Diff
Patch (2.85 KB, patch)
2011-02-28 17:01 PST, David Levin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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