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

ChromoteSession$screenshot() triggers a page resize before screenshot. #96

Open
keatonwilson opened this issue Jun 1, 2022 · 13 comments · Fixed by rstudio/shinytest2#231
Labels
bug an unexpected problem or unintended behavior

Comments

@keatonwilson
Copy link

Hi there!

Was working through building out some new testing frameworks on some of our apps and ran into some trouble developing simple snapshots. We're currently using bs4Dash for a lot of our app layouts, and it looks like there may be some compatibility issues with the snapshotting features with using this layout. Wondering if anyone has had these same issues and if it might be possible to diagnose.

I've developed a small reprex to illustrate the issue, which can be viewed (and forked) here.

In short, it appears as though there are small differences in the margins/re-sizing of elements when snapshots are taken that are generating differences between the reference images and the new snapshots. We've tried a few workarounds using Sys.sleep() to determine if it might be an animation wait-time issue, with little success. Any help anyone could provide would be great - thank you!

@keatonwilson
Copy link
Author

@yogat3ch - tagging you here for the easy follow.

@yogat3ch
Copy link

yogat3ch commented Jun 3, 2022

I added wait_for_idle before each screenshot, but that did not fix it. The 1px margin difference made me consider the choice of screen dimensions for the AppDriver. The odd values of pixels for the screen width (1293) causes the CSS when determining 50% screen width to land on either 646 or 647 as the column width. It’s probably influenced by minor differences in floating point values, so between runs this value lands on a different width. I changed the screen dimensions to a more typical screen HxW (800x600) and the issue resolved. Hope this helps anyone who encounters this issue!

@yogat3ch
Copy link

yogat3ch commented Jun 3, 2022

A couple of ideas for addressing this:

  • Adding to the docs about adjusting the screen height/width a little if minor margin changes prove to be an issue
  • Automatically adjusting the screen width when the AppDrive is instantiated to accommodate Bootstrap CSS % widths by default
  • Add a margin threshold argument on expect_screenshot that allows for elements to deviate within a radius of the specified pixel width before an error is thrown.

Thanks shinytest2 devs!

@schloerke
Copy link
Collaborator

  • Add a margin threshold argument on expect_screenshot that allows for elements to deviate within a radius of the specified pixel width before an error is thrown.

Related rstudio/shinytest2#220


Thanks shinytest2 devs!

😃

schloerke referenced this issue in rstudio/shinytest2 Feb 8, 2023
Using comment from https://github.com/rstudio/shinytest2/issues/218#issuecomment-1146097081 that suggests to use "round" numbers for easier division. While this isn't a perfect fix, it provides a much better opportunity to succeed at first run. If trouble still arises from rounded corners, then the `threshold` arg can be used.
@schloerke schloerke reopened this Feb 8, 2023
@schloerke
Copy link
Collaborator

So I can still reprex this issue with the latest of everything. Thank you for the reprex repo with {renv}.

The issue can be reproduced with a window that is larger than 992px wide and by calling app$get_screenshot(). (Kinda fun to see the bug live using app$view().)

For some really weird reason, Chrome DevTools makes jQuery believe the window size has changed, causing $(window).resize(FUNC) to trigger here: https://github.com/ColorlibHQ/AdminLTE/blob/75deb497b39c1a8547fe0f21a2478b066b223396/build/js/PushMenu.js#L160-L162

This calls: https://github.com/ColorlibHQ/AdminLTE/blob/75deb497b39c1a8547fe0f21a2478b066b223396/build/js/PushMenu.js#L107-L125 ... which believes the window width is 0, causing the sidebar to collapse, which moves the cards to the left. Immediately after this, the window is resized back to where it was and the resize method is called again, prompting the sidebar to expand and the cards to move back.

Work around:

If the AdminLTE3 can disable the auto collapse by setting the push menu option autoCollapseSize=false, then this odd behavior can be avoided. Docs: https://adminlte.io/docs/3.1/javascript/push-menu.html

This is not a fix, but there is no longer any {chromote} code that is making a resized viewport to trigger the jQuery resize. This is mind boggling to me.

I don't know of a permanent fix as there is nothing that we are doing. I'd like to point the finger at the Chrome DevTools.

@yogat3ch, please let me know if this autoCollapseSize can be disabled when shiny is in testing mode (getOption("shiny.testmode", FALSE)) (or however you see fit). Thank you!

@schloerke
Copy link
Collaborator

schloerke commented Feb 9, 2023

If the option can't be set, AdminLTE3 could be patched and have the resize method be debounced or throttled. This would avoid the really really small time domain where the window believes it has a width of 1px and a height of 1px.

@schloerke
Copy link
Collaborator

schloerke commented Feb 9, 2023

Small reprex app to get the resize to show up:

app <- 
  shinytest2::AppDriver$new(
    shiny::shinyApp(
      ui = shiny::fluidPage(
        shiny::tags$h1("Does it resize?", style="height:400px;width:400px;background-color:lightblue;"),
        shiny::tags$script("
          $(function() {
            $(window).resize(function(){
              console.log('resize triggered! Current window size: ', $(window).height(), 'x', $(window).width())
            })
          })
        ")
      ),
      server = function(...) {}
    )
  ) 
# app$view()
app$get_screenshot() # Ignoring result
app$get_logs()
#> .... (Previous logs removed for brevity)
#> {chromote}   JS info   09:18:03.45 shinytest2; Shiny has been idle for 200ms
#> {shinytest2} R  info   09:18:03.45 Shiny app started
#> {shinytest2} R  info   09:18:03.78 Viewing chromote session
#> {shinytest2} R  info   09:18:11.92 Taking screenshot
{chromote}   JS log    10:14:30.34 resize triggered! Current window size:  1 x 1
                                   (anonymous) @ :22:22
                                   dispatch    @ jquery-3.6.0/jquery.min.js:1:43063
                                   v.handle    @ jquery-3.6.0/jquery.min.js:1:41047
{chromote}   JS log    10:14:30.35 resize triggered! Current window size:  1323 x 992
                                   (anonymous) @ :22:22
                                   dispatch    @ jquery-3.6.0/jquery.min.js:1:43063
                                   v.handle    @ jquery-3.6.0/jquery.min.js:1:41047
{shiny}      R  stderr ----------- Loading required package: shiny
{shiny}      R  stderr ----------- Running application in test mode.
{shiny}      R  stderr -----------
{shiny}      R  stderr ----------- Listening on http://127.0.0.1:7415

resize triggered! Current window size: 1

^^ That's an issue since we do not believe {chromote} v0.1.1 is performing any resizing.

**Investigating

@schloerke
Copy link
Collaborator

Moving this issue as it is an underlying issue in {chromote}, not {shinytest2}.

@schloerke schloerke transferred this issue from rstudio/shinytest2 Feb 9, 2023
@schloerke
Copy link
Collaborator

Displaying the commands with a dput(msg) inside $send_command(), I can see that the only command being sent to the protocol has the shape:

list(
  method = "Page.captureScreenshot", 
  params = list(
    clip = list(
      x = 0, y = 0, 
      width = 992, height = 430, 
      scale = 1
    ), 
    fromSurface = TRUE,
    captureBeyondViewport = TRUE
  )
)

Before the above command, no resize is triggered. After the command, two resizes are triggered: (HxW) 1x1, then the viewport size of 1323x992.

Logs when captureBeyondViewport=TRUE (current behavior)
> app$get_screenshot() # Ignoring result
list(method = "Emulation.setScrollbarsHidden", params = list(
    hidden = TRUE))
list(method = "DOM.getDocument", params = structure(list(), .Names = character(0)))
list(method = "DOM.querySelector", params = list(nodeId = 1L,
    selector = "html"))
list(method = "DOM.getBoxModel", params = list(nodeId = 3L))
list(method = "DOM.querySelectorAll", params = list(nodeId = 1L,
    selector = "html"))
list(method = "DOM.getBoxModel", params = list(nodeId = 3L))
Called from: onFulfilled(value)
Browse[1]> app$get_logs()
{shinytest2} R  info   10:25:10.42 Start AppDriver initialization
{shinytest2} R  info   10:25:10.43 Starting Shiny app
{shinytest2} R  info   10:25:11.06 Creating new ChromoteSession
{shinytest2} R  info   10:25:11.19 Navigating to Shiny app
{shinytest2} R  info   10:25:11.31 Injecting shiny-tracer.js
{chromote}   JS info   10:25:11.32 shinytest2; jQuery not found
{chromote}   JS info   10:25:11.32 shinytest2; Loaded
{shinytest2} R  info   10:25:11.32 Waiting for Shiny to become ready
{chromote}   JS info   10:25:11.39 shinytest2; jQuery found
{chromote}   JS info   10:25:11.39 shinytest2; Waiting for shiny session to connect
{chromote}   JS info   10:25:11.43 shinytest2; Connected
{shinytest2} R  info   10:25:11.43 Waiting for Shiny to become idle for 200ms within 15000ms
{chromote}   JS info   10:25:11.44 shinytest2; Waiting for Shiny to be stable
{chromote}   JS info   10:25:11.64 shinytest2; Shiny has been idle for 200ms
{shinytest2} R  info   10:25:11.64 Shiny app started
{shinytest2} R  info   10:25:13.33 Taking screenshot
{shiny}      R  stderr ----------- Loading required package: shiny
{shiny}      R  stderr ----------- Running application in test mode.
{shiny}      R  stderr -----------
{shiny}      R  stderr ----------- Listening on http://127.0.0.1:5890
{shiny}      R  stdout ----------- attaching globals: Browse[1]> c
list(method = "Page.captureScreenshot", params = list(clip = list(
    x = 0, y = 0, width = 992, height = 430, scale = 1), fromSurface = TRUE,
    captureBeyondViewport = TRUE))
Called from: onFulfilled(value)
Browse[1]> app$get_logs()
{shinytest2} R  info   10:25:10.42 Start AppDriver initialization
{shinytest2} R  info   10:25:10.43 Starting Shiny app
{shinytest2} R  info   10:25:11.06 Creating new ChromoteSession
{shinytest2} R  info   10:25:11.19 Navigating to Shiny app
{shinytest2} R  info   10:25:11.31 Injecting shiny-tracer.js
{chromote}   JS info   10:25:11.32 shinytest2; jQuery not found
{chromote}   JS info   10:25:11.32 shinytest2; Loaded
{shinytest2} R  info   10:25:11.32 Waiting for Shiny to become ready
{chromote}   JS info   10:25:11.39 shinytest2; jQuery found
{chromote}   JS info   10:25:11.39 shinytest2; Waiting for shiny session to connect
{chromote}   JS info   10:25:11.43 shinytest2; Connected
{shinytest2} R  info   10:25:11.43 Waiting for Shiny to become idle for 200ms within 15000ms
{chromote}   JS info   10:25:11.44 shinytest2; Waiting for Shiny to be stable
{chromote}   JS info   10:25:11.64 shinytest2; Shiny has been idle for 200ms
{shinytest2} R  info   10:25:11.64 Shiny app started
{shinytest2} R  info   10:25:13.33 Taking screenshot
{chromote}   JS log    10:25:17.31 resize triggered! Current window size:  1 x 1
                                   (anonymous) @ :22:22
                                   dispatch    @ jquery-3.6.0/jquery.min.js:1:43063
                                   v.handle    @ jquery-3.6.0/jquery.min.js:1:41047
{chromote}   JS log    10:25:17.32 resize triggered! Current window size:  1323 x 992
                                   (anonymous) @ :22:22
                                   dispatch    @ jquery-3.6.0/jquery.min.js:1:43063
                                   v.handle    @ jquery-3.6.0/jquery.min.js:1:41047
{shiny}      R  stderr ----------- Loading required package: shiny
{shiny}      R  stderr ----------- Running application in test mode.
{shiny}      R  stderr -----------
{shiny}      R  stderr ----------- Listening on http://127.0.0.1:5890

If captureBeyondViewport is set to FALSE, then no resize is triggered.

This seems like a bug in the Chrome DevTools.

Logs when captureBeyondViewport=FALSE
> app$get_screenshot() # Ignoring result
list(method = "Emulation.setScrollbarsHidden", params = list(
    hidden = TRUE))
list(method = "DOM.getDocument", params = structure(list(), .Names = character(0)))
list(method = "DOM.querySelector", params = list(nodeId = 1L,
    selector = "html"))
list(method = "DOM.getBoxModel", params = list(nodeId = 3L))
list(method = "DOM.querySelectorAll", params = list(nodeId = 1L,
    selector = "html"))
list(method = "DOM.getBoxModel", params = list(nodeId = 3L))
Called from: onFulfilled(value)
Browse[1]> app$get_logs()
{shinytest2} R  info   10:25:27.77 Start AppDriver initialization
{shinytest2} R  info   10:25:27.78 Starting Shiny app
{shinytest2} R  info   10:25:28.41 Creating new ChromoteSession
{shinytest2} R  info   10:25:29.29 Navigating to Shiny app
{shinytest2} R  info   10:25:29.42 Injecting shiny-tracer.js
{chromote}   JS info   10:25:29.43 shinytest2; jQuery not found
{chromote}   JS info   10:25:29.44 shinytest2; Loaded
{shinytest2} R  info   10:25:29.44 Waiting for Shiny to become ready
{chromote}   JS info   10:25:29.50 shinytest2; jQuery found
{chromote}   JS info   10:25:29.50 shinytest2; Waiting for shiny session to connect
{chromote}   JS info   10:25:29.54 shinytest2; Connected
{shinytest2} R  info   10:25:29.55 Waiting for Shiny to become idle for 200ms within 15000ms
{chromote}   JS info   10:25:29.55 shinytest2; Waiting for Shiny to be stable
{chromote}   JS info   10:25:29.76 shinytest2; Shiny has been idle for 200ms
{shinytest2} R  info   10:25:29.76 Shiny app started
{shinytest2} R  info   10:25:30.09 Taking screenshot
{shiny}      R  stderr ----------- Loading required package: shiny
{shiny}      R  stderr ----------- Running application in test mode.
{shiny}      R  stderr -----------
{shiny}      R  stderr ----------- Listening on http://127.0.0.1:7752
{shiny}      R  stdout ----------- attaching globals: Browse[1]> c
list(method = "Page.captureScreenshot", params = list(clip = list(
    x = 0, y = 0, width = 992, height = 430, scale = 1), fromSurface = TRUE,
    captureBeyondViewport = FALSE))
Called from: onFulfilled(value)
Browse[1]> app$get_logs()
{shinytest2} R  info   10:25:27.77 Start AppDriver initialization
{shinytest2} R  info   10:25:27.78 Starting Shiny app
{shinytest2} R  info   10:25:28.41 Creating new ChromoteSession
{shinytest2} R  info   10:25:29.29 Navigating to Shiny app
{shinytest2} R  info   10:25:29.42 Injecting shiny-tracer.js
{chromote}   JS info   10:25:29.43 shinytest2; jQuery not found
{chromote}   JS info   10:25:29.44 shinytest2; Loaded
{shinytest2} R  info   10:25:29.44 Waiting for Shiny to become ready
{chromote}   JS info   10:25:29.50 shinytest2; jQuery found
{chromote}   JS info   10:25:29.50 shinytest2; Waiting for shiny session to connect
{chromote}   JS info   10:25:29.54 shinytest2; Connected
{shinytest2} R  info   10:25:29.55 Waiting for Shiny to become idle for 200ms within 15000ms
{chromote}   JS info   10:25:29.55 shinytest2; Waiting for Shiny to be stable
{chromote}   JS info   10:25:29.76 shinytest2; Shiny has been idle for 200ms
{shinytest2} R  info   10:25:29.76 Shiny app started
{shinytest2} R  info   10:25:30.09 Taking screenshot
{shiny}      R  stderr ----------- Loading required package: shiny
{shiny}      R  stderr ----------- Running application in test mode.
{shiny}      R  stderr -----------
{shiny}      R  stderr ----------- Listening on http://127.0.0.1:7752
{shiny}      R  stdout ----------- attaching globals: Browse[1]> c
list(method = "Emulation.setScrollbarsHidden", params = list(
    hidden = FALSE))

@schloerke
Copy link
Collaborator

Link to repo that explores this bug in Chrome DevTools Protocol: https://github.com/schloerke/chrome-devtools-protocol-screenshot-bug

Link to bug filed with Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1414460

@schloerke
Copy link
Collaborator

schloerke commented Feb 9, 2023

@wch I can repro this unexpected behavior with chrome devtools protocol, not just {chromote}. Currently any screenshot with {chromote} causes this rapid viewport resize.

Should we revert #83? Or adopt a approach similar to https://github.com/puppeteer/puppeteer/blob/abcc1756dd434dbe27d322aa9692b7fd9858a9ca/packages/puppeteer-core/src/common/Page.ts#L1400-L1419 ?

I believe the only clean solution is that Chrome DevTools fixes itself and we maintain our current code.

@gadenbuie
Copy link
Member

gadenbuie commented Oct 26, 2023

Can we rename this issue? Something like: ChromoteSession$screenshot() triggers a page resize before screenshot.

I'm currently having to do a lot of gymnastics to avoid the consequences of the page refresh. I think we should consider one of these options for updating the screenshot method:

  1. Set captureBeyondViewport = TRUE only when it's required.
  2. Set captureBeyondViewport = FALSE under certain conditions, e.g. the viewport was directly requested.
  3. Expose captureBeyondViewport as an option in the method signature with a user beware message in the docs.
  4. Or use the puppeteer approach mentioned above

@hadley hadley changed the title bs4Dash Layout Snapshot Problems ChromoteSession$screenshot() triggers a page resize before screenshot. Jan 30, 2024
@hadley hadley added the bug an unexpected problem or unintended behavior label Jan 30, 2024
@hadley
Copy link
Member

hadley commented Jan 30, 2024

I'd suggest we take the puppeteer approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants