From cbeaa815075681459e9a1c9082ac2065c75bfa99 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 12:33:14 +0000 Subject: [PATCH 01/10] Add handler on dialog box open This handler will listen out for when dialog boxes are opened. When a CDP event is received for a dialog box opening, we will automatically dismiss it. This is the first step in dealing with dialog boxes. --- common/frame_session.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/common/frame_session.go b/common/frame_session.go index 32eac456d..791fe563d 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -259,12 +259,25 @@ func (fs *FrameSession) initEvents() { fs.onAttachedToTarget(ev) case *target.EventDetachedFromTarget: fs.onDetachedFromTarget(ev) + case *cdppage.EventJavascriptDialogOpening: + fs.onEventJavascriptDialogOpening(ev) } } } }() } +func (fs *FrameSession) onEventJavascriptDialogOpening(event *cdppage.EventJavascriptDialogOpening) { + fs.logger.Debugf("FrameSession:onEventJavascriptDialogOpening", + "sid:%v tid:%v url:%v dialogType:%s", + fs.session.ID(), fs.targetID, event.URL, event.Type) + + action := cdppage.HandleJavaScriptDialog(false) + if err := action.Do(cdp.WithExecutor(fs.ctx, fs.session)); err != nil { + fs.logger.Errorf("FrameSession:onEventJavascriptDialogOpening", "failed to dismiss dialog box: %v", err) + } +} + func (fs *FrameSession) initFrameTree() error { fs.logger.Debugf("NewFrameSession:initFrameTree", "sid:%v tid:%v", fs.session.ID(), fs.targetID) From 8b6ce5d9d0bb5579deb143b88e656fc19570c63f Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 12:43:42 +0000 Subject: [PATCH 02/10] Add a alert dialog test The test dismisses an alert dialogue box. --- tests/frame_test.go | 39 +++++++++++++++++++++++++++++++++++++++ tests/static/dialog.html | 18 ++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 tests/static/dialog.html diff --git a/tests/frame_test.go b/tests/frame_test.go index 7741feaf7..e66abb20d 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -3,6 +3,7 @@ package tests import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,3 +22,41 @@ func TestFramePress(t *testing.T) { require.Equal(t, "AbC", f.InputValue("#text1", nil)) } + +func TestFrameDismissDialogBox(t *testing.T) { + + t.Parallel() + + tests := []struct { + name string + }{ + { + name: "alert", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := newTestBrowser(t, withFileServer()) + + p := b.NewPage(nil) + + err := b.await(func() error { + opts := b.toGojaValue(struct { + WaitUntil string `js:"waitUntil"` + }{ + WaitUntil: "networkidle", + }) + b.promise(p.Goto(b.staticURL("dialog.html?dialogType="+tt.name), opts)).then(func() { + result := p.TextContent("#text", nil) + assert.EqualValues(t, "Hello World", result) + }) + + return nil + }) + require.NoError(t, err) + }) + } +} diff --git a/tests/static/dialog.html b/tests/static/dialog.html new file mode 100644 index 000000000..7bc7da465 --- /dev/null +++ b/tests/static/dialog.html @@ -0,0 +1,18 @@ + + + + + +
Hello World
+ + \ No newline at end of file From 99363fc371a53b5c6436684d4cec3c05c7855ffc Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 12:46:13 +0000 Subject: [PATCH 03/10] Add confirm dialog test This test will dismiss a confirm dialog box. --- tests/frame_test.go | 3 +++ tests/static/dialog.html | 2 ++ 2 files changed, 5 insertions(+) diff --git a/tests/frame_test.go b/tests/frame_test.go index e66abb20d..29d140a0d 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -33,6 +33,9 @@ func TestFrameDismissDialogBox(t *testing.T) { { name: "alert", }, + { + name: "confirm", + }, } for _, tt := range tests { diff --git a/tests/static/dialog.html b/tests/static/dialog.html index 7bc7da465..fcdd76545 100644 --- a/tests/static/dialog.html +++ b/tests/static/dialog.html @@ -9,6 +9,8 @@ case "alert": alert("Click accept"); break; + case "confirm": + confirm("Click accept") } From e5f961db137ff50f1de2a92634bad6dce891d396 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 12:50:39 +0000 Subject: [PATCH 04/10] Add a prompt dialog test This test will dismiss a prompt type dialog box. --- tests/frame_test.go | 3 +++ tests/static/dialog.html | 2 ++ 2 files changed, 5 insertions(+) diff --git a/tests/frame_test.go b/tests/frame_test.go index 29d140a0d..8992e64bf 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -36,6 +36,9 @@ func TestFrameDismissDialogBox(t *testing.T) { { name: "confirm", }, + { + name: "prompt", + }, } for _, tt := range tests { diff --git a/tests/static/dialog.html b/tests/static/dialog.html index fcdd76545..a509b21d6 100644 --- a/tests/static/dialog.html +++ b/tests/static/dialog.html @@ -11,6 +11,8 @@ break; case "confirm": confirm("Click accept") + case "prompt": + prompt("Add text and then click accept") } From 94271d2e3fb72287339522a5dc701e8580742cd9 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 13:18:47 +0000 Subject: [PATCH 05/10] Refactor the tests to assert a change Instead of always checking that the text field is Hello World, when a dialog box opens, it will change the textField to reflect which type of dialog appeared. --- tests/frame_test.go | 4 ++-- tests/static/dialog.html | 22 +++++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/frame_test.go b/tests/frame_test.go index 8992e64bf..cefc858f1 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -56,8 +56,8 @@ func TestFrameDismissDialogBox(t *testing.T) { WaitUntil: "networkidle", }) b.promise(p.Goto(b.staticURL("dialog.html?dialogType="+tt.name), opts)).then(func() { - result := p.TextContent("#text", nil) - assert.EqualValues(t, "Hello World", result) + result := p.TextContent("#textField", nil) + assert.EqualValues(t, tt.name+" dismissed", result) }) return nil diff --git a/tests/static/dialog.html b/tests/static/dialog.html index a509b21d6..df9b267b3 100644 --- a/tests/static/dialog.html +++ b/tests/static/dialog.html @@ -1,22 +1,30 @@ - + + +
Hello World
+
+ click here + - - -
Hello World
\ No newline at end of file From 0592242aa6082ed063c7898aabc1be04da5e035a Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 13:51:08 +0000 Subject: [PATCH 06/10] Update dialog handling for beforeunload A dialog box of type beforeunload seems to only be dismissable if we accept. This is the same solution as what playwright are doing. --- common/frame_session.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/frame_session.go b/common/frame_session.go index 791fe563d..4bf3cd4fc 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -273,6 +273,10 @@ func (fs *FrameSession) onEventJavascriptDialogOpening(event *cdppage.EventJavas fs.session.ID(), fs.targetID, event.URL, event.Type) action := cdppage.HandleJavaScriptDialog(false) + if event.Type == cdppage.DialogTypeBeforeunload { + action = cdppage.HandleJavaScriptDialog(true) + } + if err := action.Do(cdp.WithExecutor(fs.ctx, fs.session)); err != nil { fs.logger.Errorf("FrameSession:onEventJavascriptDialogOpening", "failed to dismiss dialog box: %v", err) } From 5bbfb66d38e96b29ad64dc0edf3a016184ac661e Mon Sep 17 00:00:00 2001 From: ankur22 Date: Tue, 29 Nov 2022 13:52:41 +0000 Subject: [PATCH 07/10] Add a beforeunload dialog test This test will ensure that the beforeunload dialog box is accepted when we try to reload the page. Unlike other alert boxes which appear when the page loads, beforeunload boxes appear when the page unloads. --- tests/frame_test.go | 16 ++++++++++++++-- tests/static/dialog.html | 8 +++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/frame_test.go b/tests/frame_test.go index cefc858f1..c2cf8981b 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -3,6 +3,7 @@ package tests import ( "testing" + "github.com/dop251/goja" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,7 +25,6 @@ func TestFramePress(t *testing.T) { } func TestFrameDismissDialogBox(t *testing.T) { - t.Parallel() tests := []struct { @@ -39,6 +39,9 @@ func TestFrameDismissDialogBox(t *testing.T) { { name: "prompt", }, + { + name: "beforeunload", + }, } for _, tt := range tests { @@ -55,7 +58,16 @@ func TestFrameDismissDialogBox(t *testing.T) { }{ WaitUntil: "networkidle", }) - b.promise(p.Goto(b.staticURL("dialog.html?dialogType="+tt.name), opts)).then(func() { + b.promise(p.Goto(b.staticURL("dialog.html?dialogType="+tt.name), opts)).then(func() *goja.Promise { + if tt.name == "beforeunload" { + return p.Click("#clickHere", nil) + } + + result := p.TextContent("#textField", nil) + assert.EqualValues(t, tt.name+" dismissed", result) + + return nil + }).then(func() { result := p.TextContent("#textField", nil) assert.EqualValues(t, tt.name+" dismissed", result) }) diff --git a/tests/static/dialog.html b/tests/static/dialog.html index df9b267b3..24042e6f8 100644 --- a/tests/static/dialog.html +++ b/tests/static/dialog.html @@ -3,7 +3,7 @@
Hello World

- click here + From b7125b9e570564117ae525c0b0ae9dc1f0198f0e Mon Sep 17 00:00:00 2001 From: Ankur Date: Wed, 30 Nov 2022 09:01:48 +0000 Subject: [PATCH 08/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: İnanç Gümüş --- tests/frame_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/frame_test.go b/tests/frame_test.go index c2cf8981b..542728808 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -58,7 +58,11 @@ func TestFrameDismissDialogBox(t *testing.T) { }{ WaitUntil: "networkidle", }) - b.promise(p.Goto(b.staticURL("dialog.html?dialogType="+tt.name), opts)).then(func() *goja.Promise { + pageGoto := p.Goto( + b.staticURL("dialog.html?dialogType="+tt.name), + opts, + ) + b.promise(pageGoto).then(func() *goja.Promise { if tt.name == "beforeunload" { return p.Click("#clickHere", nil) } From df85e63f373b17f615f8c5051b13cf24134987bc Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 30 Nov 2022 09:41:12 +0000 Subject: [PATCH 09/10] Add comment for beforeunload why we accept Resolves: https://github.com/grafana/xk6-browser/pull/663/files#r1035684979 --- common/frame_session.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/frame_session.go b/common/frame_session.go index 4bf3cd4fc..f7cef7037 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -272,6 +272,11 @@ func (fs *FrameSession) onEventJavascriptDialogOpening(event *cdppage.EventJavas "sid:%v tid:%v url:%v dialogType:%s", fs.session.ID(), fs.targetID, event.URL, event.Type) + // Dialog type of beforeunload needs to accept the + // dialog, instead of dismissing it. We're unable to + // dismiss beforeunload dialog boxes at the moment as + // it seems to pause the exec of any other action on + // the page. I believe this is an issue in Chromium. action := cdppage.HandleJavaScriptDialog(false) if event.Type == cdppage.DialogTypeBeforeunload { action = cdppage.HandleJavaScriptDialog(true) From 40b4a87aefa1bdd6783b4d13368ee49e596ef092 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 30 Nov 2022 10:17:32 +0000 Subject: [PATCH 10/10] Refactor dialog test to reduce noise We don't need the struct yet to define each of the tests in the table test. Instead use a slice of string. Resolves: https://github.com/grafana/xk6-browser/pull/663#discussion_r1035660729 Co-authored-by: Inanc Gumus --- tests/frame_test.go | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/tests/frame_test.go b/tests/frame_test.go index 542728808..5de752323 100644 --- a/tests/frame_test.go +++ b/tests/frame_test.go @@ -27,25 +27,15 @@ func TestFramePress(t *testing.T) { func TestFrameDismissDialogBox(t *testing.T) { t.Parallel() - tests := []struct { - name string - }{ - { - name: "alert", - }, - { - name: "confirm", - }, - { - name: "prompt", - }, - { - name: "beforeunload", - }, + tests := []string{ + "alert", + "confirm", + "prompt", + "beforeunload", } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, test := range tests { + t.Run(test, func(t *testing.T) { t.Parallel() b := newTestBrowser(t, withFileServer()) @@ -59,21 +49,21 @@ func TestFrameDismissDialogBox(t *testing.T) { WaitUntil: "networkidle", }) pageGoto := p.Goto( - b.staticURL("dialog.html?dialogType="+tt.name), + b.staticURL("dialog.html?dialogType="+test), opts, ) b.promise(pageGoto).then(func() *goja.Promise { - if tt.name == "beforeunload" { + if test == "beforeunload" { return p.Click("#clickHere", nil) } result := p.TextContent("#textField", nil) - assert.EqualValues(t, tt.name+" dismissed", result) + assert.EqualValues(t, test+" dismissed", result) return nil }).then(func() { result := p.TextContent("#textField", nil) - assert.EqualValues(t, tt.name+" dismissed", result) + assert.EqualValues(t, test+" dismissed", result) }) return nil