rstudio / shiny

Easy interactive web applications with R
https://shiny.posit.co/
Other
5.35k stars 1.87k forks source link

Error when updating slider value in PhantomJS #2587

Closed wch closed 5 years ago

wch commented 5 years ago

When testing an app with shinytest (which uses) PhantomJS, the current master version of Shiny throws an error when setting the value of the slider. This started happening after we updated to jQuery 3 (#2557).

The error happens when ion.rangeSlider calls .removeProp("tabindex") here: https://github.com/rstudio/shiny/blob/ff5377da9e9c4fe904081b44a6d29e5703b43fe7/inst/www/shared/ionrangeslider/js/ion.rangeSlider.js#L1726

According to https://api.jquery.com/removeProp/ :

With some built-in properties of a DOM element or window object, browsers may generate an error if an attempt is made to remove the property.

The tabIndex property is built-in, and most browsers apparently do not throw an error if you try to remove it, but PhantomJS does throw an error.

In the previous version of jQuery that we used (1.12.4), it wrapped the removal in a try-catch: https://github.com/jquery/jquery/blob/1.12.4/src/attributes/prop.js#L16-L26

However, in the current version that we're using (3.4.1), the try-catch is gone: https://github.com/jquery/jquery/blob/3.4.1/src/attributes/prop.js#L18-L22

I think the solution is to modify our copy of ion.rangeSlider so that it wraps the removeProp() with a try-catch.

wch commented 5 years ago

If shinytest is used to set the value of a slider, it will report this error:

> shinytest::testApp("myapp")
Running mytest.R Error in session_makeRequest(self, private, endpoint, data, params, headers) : 
  Unable to delete property.

This is what the phantomjs screenshot looks like after trying to set the slider to 30:

002