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.

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

Allow changing of auto_events for specific domains. #186

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ashbythorpe
Copy link

Fixes #144
Alternative to #156 that allows more control.

Allows auto_events to be set both dynamically and on a per-domain level in Chromote and ChromoteSession. Does not change any default behaviour except for get_auto_events(), which returns a character vector of domains rather than a boolean.

Examples:

# By default, auto_events depends on the parent Chromote object
session <- ChromoteSession$new()

session$get_auto_events()
#>  [1] "Accessibility"        "Animation"            "Audits"              
#>  [4] "Autofill"             "CSS"                  "Cast"                
#>  [7] "DOM"                  "DOMSnapshot"          "DOMStorage"          
#> [10] "Database"             "HeadlessExperimental" "IndexedDB"           
#> [13] "Inspector"            "LayerTree"            "Log"                 
#> [16] "Network"              "Overlay"              "Page"                
#> [19] "Performance"          "PerformanceTimeline"  "Security"            
#> [22] "ServiceWorker"        "Fetch"                "WebAudio"            
#> [25] "WebAuthn"             "Media"                "DeviceAccess"        
#> [28] "Preload"              "FedCm"                "BluetoothEmulation"  
#> [31] "Console"              "Debugger"             "HeapProfiler"        
#> [34] "Profiler"             "Runtime"

# Disable auto_events for all domains
session$disable_auto_events()

session$get_auto_events()
#> character(0)

# Enable auto_events for just the Network domain
session$enable_auto_events("Network")

session$get_auto_events()
#> [1] "Network"

# Depend on the parent again
session$enable_auto_events(NULL)

session$get_auto_events()
#>  [1] "Accessibility"        "Animation"            "Audits"              
#>  [4] "Autofill"             "CSS"                  "Cast"                
#>  [7] "DOM"                  "DOMSnapshot"          "DOMStorage"          
#> [10] "Database"             "HeadlessExperimental" "IndexedDB"           
#> [13] "Inspector"            "LayerTree"            "Log"                 
#> [16] "Network"              "Overlay"              "Page"                
#> [19] "Performance"          "PerformanceTimeline"  "Security"            
#> [22] "ServiceWorker"        "Fetch"                "WebAudio"            
#> [25] "WebAuthn"             "Media"                "DeviceAccess"        
#> [28] "Preload"              "FedCm"                "BluetoothEmulation"  
#> [31] "Console"              "Debugger"             "HeapProfiler"        
#> [34] "Profiler"             "Runtime"

@gadenbuie
Copy link
Member

After reading #144, I would have probably attempted to fix it by doing what you proposed in #156 -- avoid calling enable or disable when the event was manually enabled (or, alternatively phrased, only automatically enable/disable when if not already manually enabled).

This PR takes an alternate approach of turning on or off auto events for a specific domain, e.g. all Page events or Fetch events.

Alternative to #156 that allows more control.

Do you prefer the domain-specific enabling/disabling over avoiding the automatic enable/disable? Or should we do both? For my own context and learning, can you compare the approaches a bit?

@ashbythorpe
Copy link
Author

From what I remember, the reason I made this PR is because I was having trouble figuring out what to do when enable()/disable() were manually called when the callback count was more than zero.

This approach gives a bit more control, and doesn't have the same problems, but is more complicated and requires more manual work from the user.

Both of them work for my specific use case. I personally prefer the first approach (in the other PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow enable() methods to be called manually
2 participants