Issue 2866013: The ChromeFrame JavascriptWindowOpen test tries to open a popup in the onload...

Issue 2866013: The ChromeFrame JavascriptWindowOpen test tries to open a popup in the onload... (Closed)

Created:
10 years, 6 months ago by ananta

Modified:
9 years, 6 months ago

Reviewers:
amit

CC:
chromium-reviews, amit

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

The ChromeFrame JavascriptWindowOpen test tries to open a popup in the onload notification, which is blocked on IE7/8 by the popup blocker thus causing this test to consistently fail. Fix is to open the popup in the ondblclick notification and to fire dummy mouse down events to the IE window to trigger this event. TBR=amit Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50458

Patch Set 1 #

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+14 lines, -8 lines)			Patch
M	chrome_frame/test/data/no_interference/window_open.html	View	1 chunk	+3 lines, -2 lines	0 comments	Download
M	chrome_frame/test/no_interference_test.cc	View	1 chunk	+6 lines, -1 line	0 comments	Download
M	chrome_frame/test/test_mock_with_web_server.h	View	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome_frame/test/test_mock_with_web_server.cc	View	1 chunk	+0 lines, -5 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

amit

10 years, 6 months ago (2010-06-22 15:53:47 UTC) #2

nice catch, lgtm.

On Tue, Jun 22, 2010 at 7:55 AM, <ananta@chromium.org> wrote:

> Reviewers: amit,
>
> Description:
> The ChromeFrame JavascriptWindowOpen test tries to open a popup in the
> onload
> notification, which is blocked on
> IE7/8 by the popup blocker thus causing this test to consistently fail. Fix
> is
> to open the popup in the ondblclick
> notification and to fire dummy mouse down events to the IE window to
> trigger
> this event.
>
> TBR=amit
>
>
> Please review this at http://codereview.chromium.org/2866013/show
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome_frame/test/data/no_interference/window_open.html
>  M     chrome_frame/test/no_interference_test.cc
>  M     chrome_frame/test/test_mock_with_web_server.h
>  M     chrome_frame/test/test_mock_with_web_server.cc
>
>
> Index: chrome_frame/test/data/no_interference/window_open.html
> ===================================================================
> --- chrome_frame/test/data/no_interference/window_open.html     (revision
> 50265)
> +++ chrome_frame/test/data/no_interference/window_open.html     (working
> copy)
> @@ -1,12 +1,13 @@
>  <html>
>   <head>
>     <script type="text/javascript">
> -      window.onload = function() {
> +      function popup() {
>         window.open("empty.html", "test");
>       }
>     </script>
>   </head>
> -  <body>
> +
> +  <body ondblclick="popup();">
>     Calling window.open...
>   </body>
>  </html>
> \ No newline at end of file
> Index: chrome_frame/test/no_interference_test.cc
> ===================================================================
> --- chrome_frame/test/no_interference_test.cc   (revision 50265)
> +++ chrome_frame/test/no_interference_test.cc   (working copy)
> @@ -136,7 +136,12 @@
>       testing::StrictMock<MockWebBrowserEventSink> > new_window_mock;
>
>   mock_.ExpectNavigationInIE(kWindowOpenUrl);
> -  EXPECT_CALL(mock_, OnIELoad(testing::StrCaseEq(kWindowOpenUrl)));
> +  EXPECT_CALL(mock_, OnIELoad(testing::StrCaseEq(kWindowOpenUrl)))
> +      .WillOnce(testing::DoAll(
> +          DelaySendMouseClickToIE(&mock_, &loop_, 0, 100, 100,
> +                                  simulate_input::LEFT),
> +          DelaySendMouseClickToIE(&mock_, &loop_, 0, 100, 100,
> +                                  simulate_input::LEFT)));
>
>   mock_.ExpectNewWindowWithIE(empty_page_url(), &new_window_mock);
>   EXPECT_CALL(new_window_mock,
> OnIELoad(testing::StrCaseEq(empty_page_url())))
> Index: chrome_frame/test/test_mock_with_web_server.cc
> ===================================================================
> --- chrome_frame/test/test_mock_with_web_server.cc      (revision 50416)
> +++ chrome_frame/test/test_mock_with_web_server.cc      (working copy)
> @@ -203,11 +203,6 @@
>       &MockWebBrowserEventSink::SendMouseClick, x, y, button), delay);
>  }
>
> -ACTION_P4(DelaySendChar, loop, delay, c, mod) {
> -  loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
> -      simulate_input::SendCharA, c, mod), delay);
> -}
> -
>  ACTION_P3(DelaySendString, loop, delay, str) {
>   loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
>       simulate_input::SendStringW, str), delay);
> Index: chrome_frame/test/test_mock_with_web_server.h
> ===================================================================
> --- chrome_frame/test/test_mock_with_web_server.h       (revision 50265)
> +++ chrome_frame/test/test_mock_with_web_server.h       (working copy)
> @@ -123,6 +123,11 @@
>   EXPECT_EQ(actual_height, height);
>  }
>
> +ACTION_P4(DelaySendChar, loop, delay, c, mod) {
> +  loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
> +      simulate_input::SendCharA, c, mod), delay);
> +}
> +
>  }  // namespace chrome_frame_test
>
>  #endif  // CHROME_FRAME_TEST_MOCK_WITH_WEB_SERVER_H_
>
>
>

Expand Messages | Collapse Messages