Skip to content

WooCommerce Services JITM: use redirect parameter.#9239

Merged
dereksmart merged 1 commit intomasterfrom
fix/wcs-jitm-no-referer-redirect
Apr 24, 2018
Merged

WooCommerce Services JITM: use redirect parameter.#9239
dereksmart merged 1 commit intomasterfrom
fix/wcs-jitm-no-referer-redirect

Conversation

@jeffstieler
Copy link
Copy Markdown
Contributor

Changes proposed in this Pull Request:

  • In WooCommerce Services JITM: Use the redirect JITM parameter, only using HTTP referer as a fallback.

Relying on referer alone fails in browsers that strip those headers for DNT purposes.

Testing instructions:

  • In a browser that doesn't send HTTP referer (like Firefox), verify that the CTA in the Install/Activate WCS JITM does not properly redirect
    • JITM is shown to administrators on the WooCommerce > Orders page if WooCommerce is active and WooCommerce Services is either uninstalled, or inactive
  • Check out this branch
  • Verify that the CTA redirects back to the originating page

Proposed changelog entry for your changes:

… referer.

Relying on referer alone fails in browsers that strip those headers for DNT purposes.
@jeffstieler jeffstieler added the [Status] Needs Review This PR is ready for review. label Apr 5, 2018
@jeffstieler jeffstieler requested a review from a team April 5, 2018 05:08
@jeffstieler jeffstieler requested a review from a team as a code owner April 5, 2018 05:08
@jeherve jeherve added [Type] Janitorial [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. labels Apr 5, 2018
@oskosk oskosk added this to the 6.1 milestone Apr 13, 2018
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Apr 24, 2018
@dereksmart dereksmart merged commit 293300e into master Apr 24, 2018
@dereksmart dereksmart deleted the fix/wcs-jitm-no-referer-redirect branch April 24, 2018 17:30
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 24, 2018
kbrown9 added a commit that referenced this pull request Oct 14, 2019
In PR #9239, wp_get_referer() was described as unrealiable because some browsers strip
the 'referer' header. So let's just use admin_url() as the fallback redirect URL if
$_GET['redirect'] is empty.
kraftbj pushed a commit that referenced this pull request Oct 16, 2019
In PR #9239, wp_get_referer() was described as unrealiable because some browsers strip
the 'referer' header. So let's just use admin_url() as the fallback redirect URL if
$_GET['redirect'] is empty.
jeherve added a commit that referenced this pull request Oct 17, 2019
* WooCommerce Services: Use home_url to create redirect url

The redirect URL is created using the value of $_SERVER['REQUEST_URI'], which includes
'wp-admin/'. When admin_url() is used to create the redirect URL, the resulting URL is
incorrect. Use home_url() instead. Also properly unslash and sanitize $_GET['redirect'].

* WooCommerce Services: Use admin_url as the fallback URL

In PR #9239, wp_get_referer() was described as unrealiable because some browsers strip
the 'referer' header. So let's just use admin_url() as the fallback redirect URL if
$_GET['redirect'] is empty.

* WooCommerce Services: Fix PHPCS errors and warnings.

* Fix last PHPCS warning and add file to whitelist.


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Type] Janitorial

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants