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

Create private content version cookie when required #6406

Closed
wants to merge 1 commit into from
Closed

Create private content version cookie when required #6406

wants to merge 1 commit into from

Conversation

moloughlin
Copy link

@moloughlin moloughlin commented Aug 31, 2016

The private-content-version cookie is required for the front-end JS to AJAX fetch private block content. Relevant JS snippet from page-cache.js:95:

  _create: function () {
            var placeholders,
                version = $.mage.cookies.get(this.options.versionCookieName);

            if (!version) {
                return;
            }
            placeholders = this._searchPlaceholders(this.element.comments());

            if (placeholders && placeholders.length) {
                this._ajax(placeholders, version);
            }
  }

Current behaviour dictates that only POST requests will generate the cookie, leaving sessions initially lacking it and causing private content to remain unloaded. See issue #3890 for one case where this is causing problems, I've added some steps to reproduce as a comment

Note: This change conforms with the function's phpdoc (see below), which up until now has not been true. "Set cookie if it is not set" was never implemented but clearly was part of the intended design.

    /**
     * Handle private content version cookie
     * Set cookie if it is not set.
     * Increment version on post requests.
     * In all other cases do nothing.
     *
     * @return void
     */

…AX fetch private block content.

Current behaviour dictates that only POST requests will generate the cookie, leaving sessions initially lacking
it and causing private content to remain unloaded. See also: issue #3890.
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Aug 31, 2016

CLA assistant check
All committers have signed the CLA.

@Ctucker9233
Copy link

What's the status on this?

@moloughlin
Copy link
Author

AFAIK no action has been taken and this issue is still not addressed in the current develop branch.

@mikeymike
Copy link

mikeymike commented Sep 29, 2016

@moloughlin correct me if I'm wrong but this wouldn't work with Varnish would it. I mean a user being served a cached page will never be able to have the cookie set right?

It looks like the built in FPC will set the cookie so this will work under those circumstances but I feel like it may also require a change in the JS to ensure customers receiving a cached page from Varnish still load the data.

Note: I've not actually tested this with Varnish, it's just a hunch

@vrann vrann self-assigned this Mar 23, 2017
@vrann vrann added this to the March 2017 milestone Mar 23, 2017
@okorshenko okorshenko modified the milestones: March 2017, April 2017 Apr 2, 2017
@okorshenko okorshenko modified the milestones: April 2017, May 2017 May 9, 2017
@vrann
Copy link
Contributor

vrann commented May 19, 2017

@mikeymike it should work because same code is executed for the Varnish in the \Magento\PageCache\Model\App\FrontController\VarnishPlugin::afterDispatch

@vrann
Copy link
Contributor

vrann commented May 19, 2017

@moloughlin can you please fix Travis failures? I.e.

.......................................PHP Fatal error:  Call to a member function setDuration() on null in /home/travis/build/magento/magento2/lib/internal/Magento/Framework/App/PageCache/Version.php on line 80
Fatal error: Call to a member function setDuration() on null in /home/travis/build/magento/magento2/lib/internal/Magento/Framework/App/PageCache/Version.php on line 80

@okorshenko okorshenko modified the milestones: May 2017, June 2017 Jun 1, 2017
@okorshenko
Copy link
Contributor

Closing doe to inactivity

@okorshenko okorshenko closed this Jun 13, 2017
magento-engcom-team pushed a commit that referenced this pull request Dec 2, 2020
[Arrows] Fixes for 2.4 (pr112) (2.4-develop)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants