Codecov Report

Merging #2596 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2596   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files           5        5           
  Lines         165      165           
=======================================
  Hits          157      157           
  Misses          8        8           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef28146...6adcf3c. Read the comment docs.

Hi,

i stumbled over your pull request as i have the same issue.
In my case i use a cross domain parent page for an iframe, where your implementation might break as well.
It's an assumption, i did not run your code, just set a cookie using chrome web developer tools within the iframe context.

To fix my case, i have to use SameSite=None and set the Secure flag:

document.cookie = 'cookietest=1; SameSite=None; Secure';

I assume your pull request will only fix cases where the iframe parent has the same origin.

Hi @dev-rke may you elaborate on that? Because the cookie we set it should only be used for Modernizr purposes (actually it gets deleted after the test), moreover, setting the Secure flag would break compatibility with non-HTTPS websites.

EDIT: Sorry for the late response

Hi @Markel ,

i did not use the whole pull request itself, but i used the main parts of the code (excluding Modernizr plugin setup code).
So i cannot assure if your implementation will work, as i did not test it.
My setup uses a cross domain iframe, so top domain and iframe domain are different. Both domains use HTTPS.
The cookie is set within the iframe. Using Chromium 86 on Ubuntu.

Setting document.cookie manually by using your code snippet with SameSite=Lax won't work within my setup, i had to use SameSite=None and the Secure flag to get it working.

Just as update: i disabled the modernizr cookie test in my specific cross domain case, as it turns out that it seem not to work reliable, it might have other causes (maybe caching).
I'd recommend not to worry too much about SameSite=Lax vs None at the moment, both approaches will be an improvement. :-)