Summary: | Referer is not sent when going back to an uncached page (blank page at kin.naver.com) | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jungshik Shin <jshin> | ||||||||||
Component: | History | Assignee: | Eric Roman <eroman> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | andersca, ap, eroman | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 525.x (Safari 3.1) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
URL: | http://kin.naver.com/list/list_kinup.php?d1id=6&dir_id=601 | ||||||||||||
Bug Depends on: | 13631 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Jungshik Shin
2008-09-12 11:53:22 PDT
FYI : Naver is #1 portal in Korea with ~70% market share. 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. 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 .... 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?)
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. Created attachment 25032 [details]
Send referrer on history navigations.
Added layout test.
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 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.
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. 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 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.
ping. Any feedback on this? Comment on attachment 26543 [details]
Send referrer on history navigations.
This looks fine to me. r=me.
So, this was landed a while ago, marking as FIXED. |