Merge pull request #299 from grafana/fix/innertext-innerhtml

Fix `{Page,Frame}.{innerHTML,innerText,textContent}` panic

Author: Ivan Mirić

common/frame.go

@@ -975,6 +975,7 @@ func (f *Frame) Hover(selector string, opts goja.Value) {
 	applySlowMo(f.ctx)
 }
 
+// InnerHTML returns the innerHTML attribute of the element located by selector.
 func (f *Frame) InnerHTML(selector string, opts goja.Value) string {
 	f.log.Debugf("Frame:InnerHTML", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)
 
@@ -995,13 +996,24 @@ func (f *Frame) InnerHTML(selector string, opts goja.Value) string {
 	}
 
 	applySlowMo(f.ctx)
-	return value.(string)
+
+	if value == nil {
+		return ""
+	}
+
+	val, ok := value.(goja.Value)
+	if !ok {
+		k6Throw(f.ctx, "unexpected innerHTML value type: %T", value)
+	}
+
+	return val.ToString().String()
 }
 
+// InnerText returns the innerText attribute of the element located by selector.
 func (f *Frame) InnerText(selector string, opts goja.Value) string {
 	f.log.Debugf("Frame:InnerText", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)
 
-	parsedOpts := NewFrameInnerHTMLOptions(f.defaultTimeout())
+	parsedOpts := NewFrameInnerTextOptions(f.defaultTimeout())
 	if err := parsedOpts.Parse(f.ctx, opts); err != nil {
 		k6Throw(f.ctx, "%w", err)
 	}
@@ -1018,7 +1030,17 @@ func (f *Frame) InnerText(selector string, opts goja.Value) string {
 	}
 
 	applySlowMo(f.ctx)
-	return value.(string)
+
+	if value == nil {
+		return ""
+	}
+
+	val, ok := value.(goja.Value)
+	if !ok {
+		k6Throw(f.ctx, "unexpected innerText value type: %T", value)
+	}
+
+	return val.ToString().String()
 }
 
 func (f *Frame) InputValue(selector string, opts goja.Value) string {
@@ -1403,6 +1425,7 @@ func (f *Frame) Tap(selector string, opts goja.Value) {
 	applySlowMo(f.ctx)
 }
 
+// TextContent returns the textContent attribute of the element located by selector.
 func (f *Frame) TextContent(selector string, opts goja.Value) string {
 	f.log.Debugf("Frame:TextContent", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)
 
@@ -1423,7 +1446,17 @@ func (f *Frame) TextContent(selector string, opts goja.Value) string {
 	}
 
 	applySlowMo(f.ctx)
-	return value.(string)
+
+	if value == nil {
+		return ""
+	}
+
+	val, ok := value.(goja.Value)
+	if !ok {
+		k6Throw(f.ctx, "unexpected textContent value type: %T", value)
+	}
+
+	return val.ToString().String()
 }
 
 func (f *Frame) Title() string {

tests/page_test.go

@@ -41,6 +41,13 @@ type emulateMediaOpts struct {
 	ReducedMotion string `js:"reducedMotion"`
 }
 
+type jsFrameBaseOpts struct {
+	Timeout string
+	Strict  bool
+}
+
+const sampleHTML = `<div><b>Test</b><ol><li><i>One</i></li></ol></div>`
+
 func TestPageEmulateMedia(t *testing.T) {
 	tb := newTestBrowser(t)
 	p := tb.NewPage(nil)
@@ -106,18 +113,8 @@ func TestPageEvaluate(t *testing.T) {
 			t.Run(tc.name, func(t *testing.T) {
 				t.Parallel()
 
-				// Not using assert.Panics* because we want to match on an error substring.
 				defer func() {
-					r := recover()
-					require.NotNil(t, r)
-					require.IsType(t, &goja.Object{}, r)
-					gotObj, _ := r.(*goja.Object)
-					got := gotObj.Export()
-					expErr := fmt.Errorf("%w", errors.New(tc.errMsg))
-					require.IsType(t, expErr, got)
-					gotErr, ok := got.(error)
-					require.True(t, ok)
-					assert.Contains(t, gotErr.Error(), expErr.Error())
+					assertPanicErrorContains(t, recover(), tc.errMsg)
 				}()
 
 				tb := newTestBrowser(t)
@@ -182,6 +179,111 @@ func TestPageGotoWaitUntilDOMContentLoaded(t *testing.T) {
 	assert.EqualValues(t, "DOMContentLoaded", actual[0], `expected "DOMContentLoaded" event to have fired`)
 }
 
+func TestPageInnerHTML(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ok", func(t *testing.T) {
+		t.Parallel()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, `<b>Test</b><ol><li><i>One</i></li></ol>`, p.InnerHTML("div", nil))
+	})
+
+	t.Run("err_empty_selector", func(t *testing.T) {
+		t.Parallel()
+
+		defer func() {
+			assertPanicErrorContains(t, recover(), "The provided selector is empty")
+		}()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.InnerHTML("", nil)
+		t.Error("did not panic")
+	})
+
+	t.Run("err_wrong_selector", func(t *testing.T) {
+		t.Parallel()
+
+		tb := newTestBrowser(t)
+		p := tb.NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, "", p.InnerHTML("p", tb.rt.ToValue(jsFrameBaseOpts{
+			Timeout: "100",
+		})))
+	})
+}
+
+func TestPageInnerText(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ok", func(t *testing.T) {
+		t.Parallel()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, "Test\nOne", p.InnerText("div", nil))
+	})
+
+	t.Run("err_empty_selector", func(t *testing.T) {
+		t.Parallel()
+
+		defer func() {
+			assertPanicErrorContains(t, recover(), "The provided selector is empty")
+		}()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.InnerText("", nil)
+		t.Error("did not panic")
+	})
+
+	t.Run("err_wrong_selector", func(t *testing.T) {
+		t.Parallel()
+
+		tb := newTestBrowser(t)
+		p := tb.NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, "", p.InnerText("p", tb.rt.ToValue(jsFrameBaseOpts{
+			Timeout: "100",
+		})))
+	})
+}
+
+func TestPageTextContent(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ok", func(t *testing.T) {
+		t.Parallel()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, "TestOne", p.TextContent("div", nil))
+	})
+
+	t.Run("err_empty_selector", func(t *testing.T) {
+		t.Parallel()
+
+		defer func() {
+			assertPanicErrorContains(t, recover(), "The provided selector is empty")
+		}()
+
+		p := newTestBrowser(t).NewPage(nil)
+		p.TextContent("", nil)
+		t.Error("did not panic")
+	})
+
+	t.Run("err_wrong_selector", func(t *testing.T) {
+		t.Parallel()
+
+		tb := newTestBrowser(t)
+		p := tb.NewPage(nil)
+		p.SetContent(sampleHTML, nil)
+		assert.Equal(t, "", p.TextContent("p", tb.rt.ToValue(jsFrameBaseOpts{
+			Timeout: "100",
+		})))
+	})
+}
+
 func TestPageInputValue(t *testing.T) {
 	p := newTestBrowser(t).NewPage(nil)
 
@@ -535,3 +637,17 @@ func TestPageWaitForNavigationShouldNotPanic(t *testing.T) {
 	<-ctx.Done()
 	require.NotPanics(t, func() { p.WaitForNavigation(nil) })
 }
+
+func assertPanicErrorContains(t *testing.T, err interface{}, expErrMsg string) {
+	t.Helper()
+
+	require.NotNil(t, err)
+	require.IsType(t, &goja.Object{}, err)
+	gotObj, _ := err.(*goja.Object)
+	got := gotObj.Export()
+	expErr := fmt.Errorf("%w", errors.New(expErrMsg))
+	require.IsType(t, expErr, got)
+	gotErr, ok := got.(error)
+	require.True(t, ok)
+	assert.Contains(t, gotErr.Error(), expErr.Error())
+}