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

Safari 13: Viewer query params cause document.referrer downgrade #25179

Closed
dreamofabear opened this issue Oct 21, 2019 · 2 comments · Fixed by #25181
Closed

Safari 13: Viewer query params cause document.referrer downgrade #25179

dreamofabear opened this issue Oct 21, 2019 · 2 comments · Fixed by #25181

Comments

@dreamofabear
Copy link

Avoids Document.referrer downgrade due to harmless viewer query params like amp_js_v and usqp.

/cc @ampproject/wg-analytics

@dreamofabear
Copy link
Author

Removing query params from the viewer iframe works, but they remain missing on back navigation in some cases. Missing query params affects some AMP functionality e.g. QUERY_PARAM() URL variable replacement.

I also did a quick code search for parseQueryString() and location.search and found the following usages:

extensions/amp-access-scroll/0.1/scroll-impl.js
extensions/amp-access/0.1/amp-login-done-dialog.js
extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
extensions/amp-analytics/0.1/linker-reader.js
src/impression.js
src/mode.js
src/service/document-info-impl.js
src/service/url-replacements-impl.js
src/service/viewer-impl.js
src/url.js

One solution that was suggested is to store the query params in memory before removal e.g. fix the above usages.

A simpler solution is to just restore the query params to the viewer iframe after the navigation event -- either on pageshow event when back navigation triggers the page cache (bfcache) or on next event loop for _blank navigation.

@dreamofabear dreamofabear changed the title Safari 13: Remove viewer query params on navigation Safari 13: Viewer query params cause document.referrer downgrade Oct 24, 2019
@zhouyx
Copy link
Contributor

zhouyx commented Oct 25, 2019

🎉 Woohoo! Thank you @choumx

cc @zikas

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

Successfully merging a pull request may close this issue.

2 participants