Enable filtering visible nodes as an option #636
Conversation
|
Creating this as a draft, since I haven't finished the documentation; if you think this approach is fine, I will add it |
Currently, we cannot tell ChromeDP to select all visible nodes without waiting for them. If one calls ``` NodeIDs(selector, WaitVisible, AtLeast(0)) ``` chromeDP selects all nodes with the selector and then wait for them to be visible. This might hang if it never happens and the node keeps being hidden. With this PR, we can do ``` NodeIDs(selector, FilterVisible, AtLeast(0)) ``` However, the following will still hang, since the waiting is done before the filtering; as we need the nodes to be at least ready to filter them. I think it's OK, but it might be confusing. ``` NodeIDs(selector, WaitVisible, FilterVisible, AtLeast(0)) ```
|
One way to fix the confusing issue that I wrote below is to have two waiting functions; one NodeReady that will be called always and not user settable, and one other that is user settable, and run after filtering. |
| @@ -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) | |||
| @@ -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 { | |||
| @@ -194,6 +195,24 @@ func (s *Selector) Do(ctx context.Context) error { | |||
| if nodes == nil || err != nil { | |||
| continue | |||
| } | |||
|
|
|||
| if s.filter != nil { | |||
There was a problem hiding this comment.
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.
| @@ -0,0 +1,38 @@ | |||
| package chromedp | |||
There was a problem hiding this comment.
this file is tiny, I would not add it. just add the funcs elsewhere like util.go, even though I hate that name.
There was a problem hiding this comment.
why are small files a problem?
Currently, we cannot tell ChromeDP to select all visible nodes without
waiting for them.
If one calls
chromeDP selects all nodes with the selector and then wait for them to be visible.
This might hang if it never happens and the node keeps being hidden.
With this PR, we can do
However, the following will still hang, since the waiting is done before the filtering;
as we need the nodes to be at least ready to filter them. I think it's OK,
but it might be confusing.