Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable filtering visible nodes as an option #636

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Enable filtering visible nodes as an option #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 36 additions & 28 deletions query.go
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@ type Selector struct {
by func(context.Context, *cdp.Node) ([]cdp.NodeID, error)
wait func(context.Context, *cdp.Frame, ...cdp.NodeID) ([]*cdp.Node, error)
after func(context.Context, ...*cdp.Node) error
filter func(context.Context, *cdp.Node) (bool, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this before wait

raw bool
}

Expand Down Expand Up @@ -194,6 +195,24 @@ func (s *Selector) Do(ctx context.Context) error {
if nodes == nil || err != nil {
continue
}

if s.filter != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we should think about moving this before "wait" in the future, but that would require an API-breaking refactor. because, ideally, the steps would be:

nodes, err := s.findNodes(...)
if nodes == nil || err != nil { continue }

if s.filter != nil { ... }
if s.wait != nil { ... }
if s.after != nil { ... }

This is useful, because imagine if you wanted to do FilterVisible, NodeEnabled to find all text inputs which are visible, then wait for them to be enabled (so that you can type into them). The order here is very important; with the current order, if there is a non-visible element which is disabled, you would first wait for it to be enabled, so you would block forever before the filter does its thing.

We don't have to do this change here, but I would add a TODO.

for i, node := range nodes {
is, err := s.filter(ctx, node)
if err != nil {
continue
}

if !is {
nodes = append(nodes[0:i], nodes[i+1:]...)
}

if len(nodes) < s.exp {
continue
}
}
}

if s.after != nil {
if err := s.after(ctx, nodes...); err != nil {
return err
Expand Down Expand Up @@ -277,6 +296,12 @@ func ByFunc(f func(context.Context, *cdp.Node) ([]cdp.NodeID, error)) QueryOptio
}
}

func FilterFunc(f func(context.Context, *cdp.Node) (bool, error)) QueryOption {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs docs

return func(s *Selector) {
s.filter = f
}
}

// ByQuery is an element query action option to select a single element by the
// DOM.querySelector command.
//
Expand Down Expand Up @@ -416,23 +441,11 @@ func NodeReady(s *Selector) {
// nodes have been sent by the browser and are visible.
func NodeVisible(s *Selector) {
WaitFunc(s.waitReady(func(ctx context.Context, n *cdp.Node) error {
// check box model
_, err := dom.GetBoxModel().WithNodeID(n.NodeID).Do(ctx)
if err != nil {
if isCouldNotComputeBoxModelError(err) {
return ErrNotVisible
}

return err
}

// check visibility
var res bool
err = EvaluateAsDevTools(snippet(visibleJS, cashX(true), s, n), &res).Do(ctx)
visible, err := isNodeVisible(ctx, n, s)
if err != nil {
return err
}
if !res {
if !visible {
return ErrNotVisible
}
return nil
Expand All @@ -443,29 +456,24 @@ func NodeVisible(s *Selector) {
// nodes have been sent by the browser and are not visible.
func NodeNotVisible(s *Selector) {
WaitFunc(s.waitReady(func(ctx context.Context, n *cdp.Node) error {
// check box model
_, err := dom.GetBoxModel().WithNodeID(n.NodeID).Do(ctx)
if err != nil {
if isCouldNotComputeBoxModelError(err) {
return nil
}

return err
}

// check visibility
var res bool
err = EvaluateAsDevTools(snippet(visibleJS, cashX(true), s, n), &res).Do(ctx)
visible, err := isNodeVisible(ctx, n, s)
if err != nil {
return err
}
if res {
if visible {
return ErrVisible
}
return nil
}))(s)
}

func FilterVisible(s *Selector) {
FilterFunc(func(ctx context.Context, n *cdp.Node) (b bool, err error) {
b, e := isNodeVisible(ctx, n, s)
return b,e
})(s)
}

// NodeEnabled is an element query option to wait until all queried element
// nodes have been sent by the browser and are enabled (ie, do not have a
// 'disabled' attribute).
Expand Down
33 changes: 33 additions & 0 deletions query_test.go
View file
Open in desktop
Expand Up @@ -87,6 +87,39 @@ func TestWaitNotVisible(t *testing.T) {
}
}


func TestFilterVisible(t *testing.T) {
t.Parallel()

ctx, cancel := testAllocate(t, "js.html")
defer cancel()

var nodeIDs []cdp.NodeID
if err := Run(ctx, NodeIDs("#input2", &nodeIDs, ByID, FilterVisible)); err != nil {
t.Fatalf("got error: %v", err)
}
if len(nodeIDs) != 1 {
t.Errorf("expected to have exactly 1 node id: got %d", len(nodeIDs))
}

var value string
if err := Run(ctx,
Click("#button2", ByID),
WaitNotVisible("#input2", ByID),
Value(nodeIDs, &value, ByNodeID),
); err != nil {
t.Fatalf("got error: %v", err)
}

if err := Run(ctx, NodeIDs("#input2", &nodeIDs, ByID, FilterVisible, AtLeast(0))); err != nil {
t.Fatalf("got error: %v", err)
}
if len(nodeIDs) != 0 {
t.Errorf("expected to have exactly 0 node id: got %d", len(nodeIDs))
}
}


func TestWaitEnabled(t *testing.T) {
t.Parallel()

Expand Down
7 changes: 0 additions & 7 deletions util.go
View file
Open in desktop
Expand Up @@ -4,7 +4,6 @@ import (
"net"
"net/url"

"github.com/chromedp/cdproto"
"github.com/chromedp/cdproto/cdp"
)

Expand Down Expand Up @@ -318,9 +317,3 @@ func removeNode(n []*cdp.Node, id cdp.NodeID) []*cdp.Node {
return append(n[:i], n[i+1:]...)
}

// isCouldNotComputeBoxModelError unwraps err as a MessageError and determines
// if it is a compute box model error.
func isCouldNotComputeBoxModelError(err error) bool {
e, ok := err.(*cdproto.Error)
return ok && e.Code == -32000 && e.Message == "Could not compute box model."
}
38 changes: 38 additions & 0 deletions visibility.go
View file
Open in desktop
@@ -0,0 +1,38 @@
package chromedp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is tiny, I would not add it. just add the funcs elsewhere like util.go, even though I hate that name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are small files a problem?


import (
"context"
"github.com/chromedp/cdproto"
"github.com/chromedp/cdproto/cdp"
"github.com/chromedp/cdproto/dom"
)

// isCouldNotComputeBoxModelError unwraps err as a MessageError and determines
// if it is a compute box model error.
func isCouldNotComputeBoxModelError(err error) bool {
e, ok := err.(*cdproto.Error)
return ok && e.Code == -32000 && e.Message == "Could not compute box model."
}

func isNodeVisible(ctx context.Context, n *cdp.Node, s *Selector) (bool, error) {
// check box model
_, err := dom.GetBoxModel().WithNodeID(n.NodeID).Do(ctx)
if err != nil {
if isCouldNotComputeBoxModelError(err) {
return false, nil
}

return false, err
}

// check visibility
var res bool
err = EvaluateAsDevTools(snippet(visibleJS, cashX(true), s, n), &res).Do(ctx)
if err != nil {
return false, err
}
if !res {
return false, nil
}
return true, nil
}