Bug 20806 - Referer is not sent when going back to an uncached page (blank page at kin.naver.com)
Summary: Referer is not sent when going back to an uncached page (blank page at kin.na...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: All All
: P2 Normal
Assignee: Eric Roman
URL: http://kin.naver.com/list/list_kinup....
Keywords:
Depends on: 13631
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-12 11:53 PDT by Jungshik Shin
Modified: 2009-01-28 22:29 PST (History)
3 users (show)

See Also:


Attachments
Preserve the request referrer across history navigations. (12.22 KB, patch)
2008-11-05 22:27 PST, Eric Roman
no flags Details | Formatted Diff | Diff
Send referrer on history navigations. (15.21 KB, patch)
2008-11-10 16:21 PST, Eric Roman
eric: review-
Details | Formatted Diff | Diff
Send referrer on history navigations. (9.83 KB, patch)
2009-01-08 14:12 PST, Eric Roman
no flags Details | Formatted Diff | Diff
Send referrer on history navigations. (14.72 KB, patch)
2009-01-08 16:22 PST, Eric Roman
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jungshik Shin 2008-09-12 11:53:22 PDT
How to reproduce:

1. Go to http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601 
2. Around the bottom of the page, there's a list of pages (1 2 3 4 5 6 
...). Click on one of them (other than '1') to go to the corresponding 
page.
3. If you click on '3', the url will be http://kin.naver.com/list/list_kinup.phpd1id=6&dir_id=601&sort=kinup_point&
page=3

4. Click on the 2nd column from the left in the list of Q&A items with 
'thumb-up' icons in the rightmost column.

5. The url will be something like this:
http://kin.naver.com/detail/detail.php?
d1id=6&dir_id=60101&eid=jbyUO8ElqrWTXbxk7ObSqBSon7Nxrg8Z&l_url=L2xpc3QvbGlz
dF9raW51cC5waHA/ZDFpZD02JmRpcl9pZD02MDEmc29ydD1raW51cF9wb2ludCZwYWdlPTM=

6. Press 'the back button'.

7. The url in the address bar is 
http://kin.naver.com/list/list_kinup.phpd1id=6&dir_id=601&sort=kinup_point&
page=3

Exactly the same as what you had in step 3

What's expected:

In step 7, get the same page as shown in step 3

Actual result:

In step 7, a blank screen is shown.   

Additional info : 
IE 7 and Firefox do not have this issue. Chrome has the same issue ( http://code.google.com/p/chromium/issues/detail?id=2189 : Chrome shows HTTP 404 error page. ). So does Safari with webkit trunk build on Mac OS 10.4.11
Comment 1 Jungshik Shin 2008-09-12 11:56:04 PDT
FYI : Naver is #1 portal in Korea with ~70% market share. 

Comment 2 Alexey Proskuryakov 2008-09-13 07:48:42 PDT
A blank page is what this server gives for the page 3 URL when loading it manually, too. Compare output of

curl -i "http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601&sort=kinup_point&page=3" --header "Referer: http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601"

and 

curl -i "http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601&sort=kinup_point&page=3"

So, the issue is two-fold. First, the page doesn't go to back/forward cache, that's bug 13631. Second, we don't re-send Referer header when going back involves a reload. I think that we should fix both.
Comment 3 Jungshik Shin 2008-09-15 14:36:01 PDT
Thanks for the diagnosis. 

So, if we go directly to 'http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601&sort=kinup_point&page=3', we'll get 404 error. 

Ick ...   I guess I know why Naver does that ....

Comment 4 Eric Roman 2008-11-05 22:27:21 PST
Created attachment 24935 [details]
Preserve the request referrer across history navigations.

This patch replaces the two referrer fields in HistoryItem (m_rssFeedReferrer, m_formReferrer) with a single generic one (m_referrer). It doesn't look like the distinction was necessary.

I am not sure how to write a test for this, ideas welcome (in particular how to avoid page cache in layout test?)
Comment 5 Alexey Proskuryakov 2008-11-05 23:02:13 PST
Page cache is currently always disabled in DumpRenderTree. There are also many examples of reasons of documents not to use it in bug 4123.

But these are all bugs that need to be fixed some day.
Comment 6 Eric Roman 2008-11-10 16:21:06 PST
Created attachment 25032 [details]
Send referrer on history navigations.

Added layout test.
Comment 7 Eric Seidel (no email) 2008-11-17 13:38:56 PST
Comment on attachment 25032 [details]
Send referrer on history navigations.

This looks good from my perspective, but I think Bradee or Anders will need to chime in.  Apple will need to do an internal code search before removing the RSSReferrer methods (which could be changed to just grab referrer() (possibly conditionally on if it's an RSS url) if necessary as a crutch.
Comment 8 Eric Seidel (no email) 2008-12-08 15:49:29 PST
Comment on attachment 25032 [details]
Send referrer on history navigations.

On second thought, I would suggest *not* removing the RSSFeedReferrer methods, since that will make this harder to review.  The Apple folks who maintain those APIs should take care of that.  You should just fix those methods to do the right thing, and then the Apple folks can come back and remove them when they're not needed.
Comment 9 Eric Roman 2009-01-08 14:12:17 PST
Created attachment 26534 [details]
Send referrer on history navigations.

> I would suggest *not* removing the RSSFeedReferrer methods,
> since that will make this harder to review.  The Apple folks who maintain those
> APIs should take care of that.

Done.
I removed the deletion of the "rss" and "form" referrers, and marked them as deprecated instead.
Comment 10 Eric Roman 2009-01-08 16:22:59 PST
Created attachment 26543 [details]
Send referrer on history navigations.

I had misunderstood Eric Seidel's previous comment.
This time it should be what he had suggested.

Do I also need to add the new symbol to WebCore.order ?

Thanks!
Comment 11 Eric Seidel (no email) 2009-01-08 16:46:08 PST
Comment on attachment 26543 [details]
Send referrer on history navigations.

Apple maintains the order files.  They'll get regenerated when necessary.  Missing symbols from an order file just won't get sorted to the front of the linked binary.

This patch looks good to me, but ideally someone from Apple would comment about if this could have unintended consequences for RSS stuff.
Comment 12 Eric Roman 2009-01-21 15:23:59 PST
ping. Any feedback on this?
Comment 13 Eric Seidel (no email) 2009-01-21 17:01:22 PST
Comment on attachment 26543 [details]
Send referrer on history navigations.

This looks fine to me.  r=me.
Comment 14 Eric Roman 2009-01-28 17:11:52 PST
http://trac.webkit.org/changeset/40132
Comment 15 Alexey Proskuryakov 2009-01-28 22:29:38 PST
So, this was landed a while ago, marking as FIXED.