diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-22 00:02:34 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-22 00:02:34 +0000 |
commit | 7e3544bd5859cad57261bc4827686f266a8d3961 (patch) | |
tree | e3859f5a3692cf096ba92f02636b46ccd24e2f08 | |
parent | 52574021ab8190a5b93bdf1344dada2b420f4b1c (diff) | |
download | chromium_src-7e3544bd5859cad57261bc4827686f266a8d3961.zip chromium_src-7e3544bd5859cad57261bc4827686f266a8d3961.tar.gz chromium_src-7e3544bd5859cad57261bc4827686f266a8d3961.tar.bz2 |
If we navigate to a URL which has an anchor in IE, which eventually ends up in ChromeFrame,
the actual URL queried from the moniker passed in to our implementation of IPersistStream::Load
does not have the anchor. It looks like there are some private interfaces exposed by MSHTML and
invoked by IEFrame to pass this information over.
Our fix is to save away the url received in our Bho BeforeNavigate2 notification and use this URL
if available before querying the moniker for the URL. This needs to be done in IPersistStream::Load
and in the OnHttpEquiv patch when we initiate a navigation to the actual URL using the moniker.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=27270
Bug=27270
Test=Covered by unit test.
Review URL: http://codereview.chromium.org/542096
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36814 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome_frame/bho.cc | 23 | ||||
-rw-r--r-- | chrome_frame/bho.h | 7 | ||||
-rw-r--r-- | chrome_frame/chrome_active_document.cc | 11 | ||||
-rw-r--r-- | chrome_frame/test/chrome_frame_unittests.cc | 14 | ||||
-rw-r--r-- | chrome_frame/test/data/fulltab_anchor_url_navigate.html | 33 | ||||
-rw-r--r-- | chrome_frame/urlmon_url_request.cc | 2 | ||||
-rw-r--r-- | chrome_frame/utils.cc | 26 | ||||
-rw-r--r-- | chrome_frame/utils.h | 12 |
8 files changed, 115 insertions, 13 deletions
diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc index f0bd64e..896ca65 100644 --- a/chrome_frame/bho.cc +++ b/chrome_frame/bho.cc @@ -147,7 +147,7 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url, } } } - + url_ = url->bstrVal; return S_OK; } @@ -268,14 +268,29 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv, // If there's a referrer, preserve it. std::wstring headers; + // Pass in URL fragments if applicable. + std::wstring fragment; + Bho* chrome_frame_bho = Bho::GetCurrentThreadBhoInstance(); - if (chrome_frame_bho && !chrome_frame_bho->referrer().empty()) { - headers = StringPrintf(L"Referer: %ls\r\n\r\n", + if (chrome_frame_bho) { + if (!chrome_frame_bho->referrer().empty()) { + headers = StringPrintf(L"Referer: %ls\r\n\r\n", ASCIIToWide(chrome_frame_bho->referrer()).c_str()); + } + // If the original URL contains an anchor, then the URL queried + // from the moniker does not contain the anchor. To workaround + // this we retrieve the URL from our BHO. + std::wstring moniker_url = GetActualUrlFromMoniker( + moniker, NULL, chrome_frame_bho->url()); + + GURL parsed_moniker_url(moniker_url); + if (parsed_moniker_url.has_ref()) { + fragment = UTF8ToWide(parsed_moniker_url.ref()); + } } hr = NavigateBrowserToMoniker(browser, moniker, headers.c_str(), - bind_context); + bind_context, fragment.c_str()); if (SUCCEEDED(hr)) { // Now that we've reissued the request, we need to remove the diff --git a/chrome_frame/bho.h b/chrome_frame/bho.h index c70097f..fdae5bf 100644 --- a/chrome_frame/bho.h +++ b/chrome_frame/bho.h @@ -90,10 +90,14 @@ END_SINK_MAP() IBrowserService* browser, IShellView* shell_view, BOOL done, VARIANT* in_arg, VARIANT* out_arg); - std::string referrer() const { + const std::string& referrer() const { return referrer_; } + const std::wstring& url() const { + return url_; + } + // Returns the Bho instance for the current thread. This is returned from // TLS. static Bho* GetCurrentThreadBhoInstance(); @@ -102,6 +106,7 @@ END_SINK_MAP() bool PatchProtocolHandler(const CLSID& handler_clsid); std::string referrer_; + std::wstring url_; static base::LazyInstance<base::ThreadLocalPointer<Bho> > bho_current_thread_instance_; diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index 79253d1..e92aa0b 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -204,9 +204,14 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable, SetClientSite(client_site); } - CComHeapPtr<WCHAR> display_name; - moniker_name->GetDisplayName(bind_context, NULL, &display_name); - std::wstring url = display_name; + Bho* chrome_frame_bho = Bho::GetCurrentThreadBhoInstance(); + + // If the original URL contains an anchor, then the URL queried + // from the moniker does not contain the anchor. To workaround + // this we retrieve the URL from our BHO. + std::wstring url = GetActualUrlFromMoniker( + moniker_name, bind_context, + chrome_frame_bho ? chrome_frame_bho->url() : std::wstring()); // The is_new_navigation variable indicates if this a navigation initiated // by typing in a URL for e.g. in the IE address bar, or from Chrome by diff --git a/chrome_frame/test/chrome_frame_unittests.cc b/chrome_frame/test/chrome_frame_unittests.cc index 5ed280b..3c9899f 100644 --- a/chrome_frame/test/chrome_frame_unittests.cc +++ b/chrome_frame/test/chrome_frame_unittests.cc @@ -1855,3 +1855,17 @@ TEST_F(ChromeFrameTestWithWebServer, ASSERT_TRUE(CheckResultFile(L"FullTab_DeleteCookieTest", "OK")); } +const wchar_t kChromeFrameFullTabModeAnchorUrlNavigate[] = + L"files/fulltab_anchor_url_navigate.html#chrome_frame"; + +TEST_F(ChromeFrameTestWithWebServer, + FullTabModeIE_ChromeFrameAnchorUrlNavigateTest) { + chrome_frame_test::TimedMsgLoop loop; + + ASSERT_TRUE(LaunchBrowser(IE, kChromeFrameFullTabModeAnchorUrlNavigate)); + + loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds); + + chrome_frame_test::CloseAllIEWindows(); + ASSERT_TRUE(CheckResultFile(L"FullTab_AnchorURLNavigateTest", "OK")); +} diff --git a/chrome_frame/test/data/fulltab_anchor_url_navigate.html b/chrome_frame/test/data/fulltab_anchor_url_navigate.html new file mode 100644 index 0000000..4987863 --- /dev/null +++ b/chrome_frame/test/data/fulltab_anchor_url_navigate.html @@ -0,0 +1,33 @@ +<html> + <head> + <meta http-equiv="x-ua-compatible" content="chrome=1" /> + <title>ChromeFrame fulltab mode anchor url navigate test</title> + <script type="text/javascript" + src="chrome_frame_tester_helpers.js"></script> + + <script type="text/javascript"> + function ValidateUserAgent() { + if (!isRunningInChrome()) { + onFailure("FullTab_AnchorURLNavigateTest", 1, + "Not running in Chrome."); + return; + } + ValidateWindowLocation(); + } + + function ValidateWindowLocation() { + if (/#chrome_frame/.test(window.location)) { + onSuccess("FullTab_AnchorURLNavigateTest", 1); + } else { + onFailed("FullTab_AnchorURLNavigateTest", 1, + "Failed to find anchor in URL"); + } + } + </script> + </head> + + <body onLoad="setTimeout(ValidateUserAgent, 100);"> + ChromeFrame full tab mode anchor URL validation test. + Validates that URLs with anchor are received correctly in Chrome. + </body> +</html> diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 0c16dc5..bf78269 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -341,7 +341,7 @@ void UrlmonUrlRequest::TransferToHost(IUnknown* host) { ScopedComPtr<IBindCtx> bind_context; CreateBindCtx(0, bind_context.Receive()); DCHECK(bind_context); - NavigateBrowserToMoniker(host, moniker_, NULL, bind_context); + NavigateBrowserToMoniker(host, moniker_, NULL, bind_context, NULL); moniker_.Release(); } } diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index 67b1217..3ef49df 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -636,7 +636,8 @@ bool IsOptInUrl(const wchar_t* url) { } HRESULT NavigateBrowserToMoniker(IUnknown* browser, IMoniker* moniker, - const wchar_t* headers, IBindCtx* bind_ctx) { + const wchar_t* headers, IBindCtx* bind_ctx, + const wchar_t* fragment) { DCHECK(browser); DCHECK(moniker); DCHECK(bind_ctx); @@ -695,7 +696,7 @@ HRESULT NavigateBrowserToMoniker(IUnknown* browser, IMoniker* moniker, hr = browser_priv2->NavigateWithBindCtx2(uri_obj, NULL, NULL, NULL, headers_var.AsInput(), bind_ctx, - NULL); + const_cast<wchar_t*>(fragment)); DLOG_IF(WARNING, FAILED(hr)) << StringPrintf(L"NavigateWithBindCtx2 0x%08X", hr); } @@ -709,7 +710,8 @@ HRESULT NavigateBrowserToMoniker(IUnknown* browser, IMoniker* moniker, ScopedVariant var_url(url); hr = browser_priv->NavigateWithBindCtx(var_url.AsInput(), NULL, NULL, NULL, headers_var.AsInput(), - bind_ctx, NULL); + bind_ctx, + const_cast<wchar_t*>(fragment)); DLOG_IF(WARNING, FAILED(hr)) << StringPrintf(L"NavigateWithBindCtx 0x%08X", hr); } else { @@ -830,3 +832,21 @@ bool IsHeadlessMode() { return headless; } +std::wstring GetActualUrlFromMoniker(IMoniker* moniker, + IBindCtx* bind_context, + const std::wstring& bho_url) { + CComHeapPtr<WCHAR> display_name; + moniker->GetDisplayName(bind_context, NULL, &display_name); + std::wstring moniker_url = display_name; + + GURL parsed_url(WideToUTF8(bho_url)); + if (!parsed_url.has_ref()) + return moniker_url; + + if (StartsWith(bho_url, moniker_url, false) && + bho_url[moniker_url.length()] == L'#') + return bho_url; + + return moniker_url; +} + diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h index 8048beb..f42062c 100644 --- a/chrome_frame/utils.h +++ b/chrome_frame/utils.h @@ -225,7 +225,8 @@ HRESULT GetUrlFromMoniker(IMoniker* moniker, IBindCtx* bind_context, // Navigates an IWebBrowser2 object to a moniker. // |headers| can be NULL. HRESULT NavigateBrowserToMoniker(IUnknown* browser, IMoniker* moniker, - const wchar_t* headers, IBindCtx* bind_ctx); + const wchar_t* headers, IBindCtx* bind_ctx, + const wchar_t* fragment); // Raises a flag on the current thread (using TLS) to indicate that an // in-progress navigation should be rendered in chrome frame. @@ -314,4 +315,13 @@ STDMETHODIMP QueryInterfaceIfDelegateSupports(void* obj, REFIID iid, extern const wchar_t kChromeFrameHeadlessMode[]; +// The urls retrieved from the IMoniker interface don't contain the anchor +// portion of the actual url navigated to. This function checks whether the +// url passed in the bho_url parameter contains an anchor and if yes checks +// whether it matches the url retrieved from the moniker. If yes it returns +// the bho url, if not the moniker url. +std::wstring GetActualUrlFromMoniker(IMoniker* moniker, + IBindCtx* bind_context, + const std::wstring& bho_url); + #endif // CHROME_FRAME_UTILS_H_ |