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

Feature Request: Option to disable HTML Compression for Logged-In Users #650

Closed
renzms opened this issue Jan 7, 2016 · 23 comments
Closed

Comments

@renzms
Copy link

renzms commented Jan 7, 2016

HTML compression allows for cached pages to load a lot faster compared to the initial visit to the page. Logged in users may have a much slower first time visit of loading of a page because of the HTML Compression. Compared to regular visitors or users who are not logged in, those visitors/users may have a faster experience because of the Auto-Cache Engine and the way it can pre-cache.

There should be an option for site owners who don't want to use HTML Compression for logged in users: Do NOT use Html Compression for logged in users?

Additionally, some site owners may choose not to use HTML Compression for certain URLs.

There should be a filter to list down specific URLs to exclude from HTML Compression.


Referencing Internal Ticket: https://websharks.zendesk.com/agent/tickets/10412

@raamdev
Copy link
Contributor

raamdev commented Jan 7, 2016

There should be an option for site owners who don't want to use HTML Compression for logged in users: Do NOT use Html Compression for logged in users?

Love that idea! Thank you.

Additionally, some site owners may choose not to use HTML Compression for certain URLs. There should be a filter to list down specific URLs to exclude from HTML Compression.

That sounds like a good idea too. I forked that to a separate GitHub issue: #651

@raamdev raamdev changed the title Feature Request: Additional options/filters for HTML Compression Feature Request: Option to disable HTML Compression for Logged-In Users Jan 7, 2016
@renzms
Copy link
Author

renzms commented Jan 8, 2016

@raamdev

Cool, thanks for separating the two as I wasn't sure if I should make a Feature Request Issue for each feature. I'd love to work on both once an outline is made!

@jaswrks
Copy link

jaswrks commented Jan 12, 2016

Another +1 for this here. Private/internal ticket ↓
https://websharks.zendesk.com/agent/tickets/10506

@renzms
Copy link
Author

renzms commented Jan 15, 2016

Next Actions (Step 1 of 2)

  • New feature branch in the websharks/zencache-pro repo.

  • After this line add the following:

    'htmlc_when_logged_in',
  • After this line add the following:

    'htmlc_when_logged_in' => '0', // `0|1`; enable when logged in?
    
  • After this line add the following:

    /**
    * @since 15xxxx Adding logged-in check.
    *
    * @type bool HTMLC when logged in?
    */
    protected $htmlc_when_logged_in;
    $this->htmlc_when_logged_in = (boolean) $this->plugin->options['htmlc_when_logged_in'];
    if (!$this->htmlc_when_logged_in && $this->plugin->isLikeUserLoggedIn()) {
        return; // Disable in this case.
    }

Step 2

Add UI option for:

'htmlc_when_logged_in'

After this line here

  • Submit PR.

@raamdev
Copy link
Contributor

raamdev commented Jan 15, 2016

@renzms Nice work getting us started on this! Thank you. 😄

This one looks wrong; I don't think that code belongs in advanced-cache.txt:


  • After this line add the following:

    /**
    * @since 15xxxx Adding logged-in check.
    *
    * @type bool HTMLC when logged in?
    */
    protected $htmlc_when_logged_in;
    $this->htmlc_when_logged_in = (boolean) $this->plugin->options['htmlc_when_logged_in'];
    if (!$this->htmlc_when_logged_in && $this->plugin->isLikeUserLoggedIn()) {
        return; // Disable in this case.
    }

There also appears to be something missing here:


Add UI option for:

'htmlc_when_logged_in'

After this line here

  • Submit PR.

@renzms
Copy link
Author

renzms commented Jan 16, 2016

Updated links to use the 160103 branch

@renzms
Copy link
Author

renzms commented Jan 16, 2016

###Attempt for Step 2

Attempt for Step 2

Add UI option for:

htmlc_when_logged_in

After this line here

After this line here

Submit PR.

echo '      <h3>'.__('Do NOT use Html Compression for logged in users?', SLUG_TD).'</h3>'."\n";
echo '      <p>'.__('HTML compression allows for cached pages to load a lot faster compared to the initial visit to the page. Logged in users may have a much slower first time visit of loading of a page because of the HTML Compression. Compared to regular visitors or users who are not logged in, those visitors/users may have a faster experience because of the Auto-Cache Engine and the way it can pre-cache. In short, do NOT turn this off unless you know what you\'re doing.', SLUG_TD).'</p>'."\n";
echo '      <p><select name="'.esc_attr(GLOBAL_NS).'[saveOptions][cdn_when_logged_in]">'."\n";
echo '            <option value="0"'.selected($this->plugin->options['htmlc_when_logged_in'], '0', false).'>'.__('Yes, disable HTML Compression for logged in users.', SLUG_TD).'</option>'."\n";
echo '            <option value="postload"'.selected($this->plugin->options['htmlc_when_logged_in'], 'postload', false).'>'.__('No, enable HTML Compression for logged in users.', SLUG_TD).'</option>'."\n";
echo '         </select></p>'."\n";

@raamdev
Copy link
Contributor

raamdev commented Jan 19, 2016

@renzms Something still looks wrong with that. ↑

@raamdev raamdev added this to the Next Release (Pro) milestone Jan 19, 2016
@renzms
Copy link
Author

renzms commented Jan 21, 2016

@raamdev

I see, I think its this line:

echo '      <p><select name="'.esc_attr(GLOBAL_NS).'[saveOptions][cdn_when_logged_in]">'."\n";

Should be:

echo '      <p><select name="'.esc_attr(GLOBAL_NS).'[saveOptions][htmlc_when_logged_in]">'."\n";

@raamdev
Copy link
Contributor

raamdev commented Jan 21, 2016

@renzms Good catch! But I was actually referring your formatting and missing link in your GitHub comment:

2016-01-21_11-05-00


Nice work on putting together the outline! Please go ahead and open a PR for the above work so that we can start reviewing code. :-)

@renzms
Copy link
Author

renzms commented Jan 22, 2016

@raamdev

Oh yeah, thanks! Edited the above. Funny, in my previous posts I actually included the link! I'll get started on the PR.

@KTS915
Copy link

KTS915 commented Jan 31, 2016

@renzms, @jaswsinc, @raamdev: I'm curious. What would be the benefit to logged-in users of disabling HTML compression?

@raamdev
Copy link
Contributor

raamdev commented Feb 3, 2016

@KTS915 If a site has CSS/JS that gets loaded for logged-in users that is not compatible with the HTML Compressor, it would be nice to have an option for disabling it.

@jaswrks
Copy link

jaswrks commented Feb 3, 2016

The issue is that cached pages for logged-in users have a much shorter TTL. For instance, if you're logged into the site and you submit a form, that triggers a cache clearing. Lots of little actions you take can result in a clearing of the cache. This shorter TTL is not ideal when running the HTML Compressor, because the HTML Compressor does a deep analysis of the page content and resources in order to compress things for you. For logged-in users, it is better to skip that and just cache the HTML source as-is, avoiding the extra overhead.

@KTS915
Copy link

KTS915 commented Feb 3, 2016

I see! Thank you both, @raamdev and @jaswsinc!

@KTS915
Copy link

KTS915 commented Feb 7, 2016

@jaswsinc wrote: "For logged-in users, it is better to skip that and just cache the HTML source as-is, avoiding the extra overhead."

That was particularly enlightening. I have been trying this out and, on every site I tried, it was indeed better to turn off the compressor for logged-in users. I would never have guessed.

So another big thanks for the explanation, and a +1 for this feature request!

@raamdev
Copy link
Contributor

raamdev commented Feb 8, 2016

@jaswsinc writes...

For logged-in users, it is better to skip that and just cache the HTML source as-is, avoiding the extra overhead.

That's a good reason to make the default for this new option "Enabled" (so that the HTML Compressor is disabled for Logged-In Users by default). The above should also be explained in the description for this option, i.e., that leaving the HTML Compressor disabled for Logged-In Users will improve performance in most cases.

@jaswrks
Copy link

jaswrks commented Feb 8, 2016

Agree.

@raamdev
Copy link
Contributor

raamdev commented Feb 13, 2016

@renzms I updated wpsharks/comet-cache-pro#208 with a few notes. Any chance you could get that PR finished by Monday evening so that this feature can be included in the next ZenCache Pro release (RC is targeted for Monday night, February 15th).

renzms added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 15, 2016
@renzms
Copy link
Author

renzms commented Feb 15, 2016

@raamdev
Updated according to your notes, thanks!

@raamdev raamdev modified the milestones: Next Release (Pro); 1st CC Notice, Future Release (Pro) Feb 16, 2016
renzms added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 25, 2016
renzms added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 25, 2016
renzms added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 25, 2016
renzms added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Feb 26, 2016
@raamdev
Copy link
Contributor

raamdev commented Feb 27, 2016

2016-02-26_09-02-43

@raamdev
Copy link
Contributor

raamdev commented Feb 27, 2016

Next Release Changelog:

  • Enhancement (Pro): A new HTML Compression options allows you to define whether or not HTML Compression should be enabled for Logged-In users (when Logged-In User caching is enabled). See Comet Cache → Plugin Options → HTML Compression → Enable HTML Compression for Logged-In Users?. Props @renzms. See Issue #650.

@raamdev raamdev closed this as completed Feb 27, 2016
@raamdev
Copy link
Contributor

raamdev commented Feb 27, 2016

Comet Cache v160227 has been released and includes changes from this GitHub Issue. See the v160227 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#650).

@wpsharks wpsharks locked and limited conversation to collaborators Feb 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants