Prevent lazyload on webcrawlers visit

1. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -16,8 +16,9 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +      // Marking the view as executed prevents the Search API from querying
   

   I think we could remove the reference to Search API. I wrote it for a project which was using SAPI but really, this just prevents the view from executing the query.

2. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +48,15 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    $crawler_agents = '/Googlebot|msnbot|Rambler|Yahoo|AbachoBOT|accoona|AcioRobot|ASPSeek|CocoCrawler|Dumbot|FAST-WebCrawler|GeonaBot|Gigabot|Lycos|MSRBOT|Scooter|AltaVista|IDBot|eStyle|Scrubby/i';
   

   Lets wrap this in a variable_get() at the very least so we can change it globally.

3. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +48,15 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    return (preg_match($crawler_agents, $USER_AGENT)) ? true : false;
   

   If this is truthy, can't we just return preg_match value and cast to bool?

OK, welcome to contributing :) Here's some docs that might help: https://www.drupal.org/novice

Usually how the process goes from a high level:

1. Someone posts a patch - Status: Needs Review.
2. Others review the patch and leave feedback. Status: Needs Work:
3. Patches continue to be posted addressing feedback, status back to needs review.
4. Once reviewers are happy issue goes to Reviewed and Tested by the Community
5 Maintainer commits the patch, sets to Fixed.

I'd also checkout the Drupal Coding Standards. Specifically in your patch I noticed the lowercase true/false where as Drupal always uses TRUE/FALSE.

Great, thanks for working on this. Few points to note in the review, mainly just look like copy and paste errors from the system module. Also, if you fancy trying, it would be great if we could add tests for this, not sure how easy it will be to set the user agency though in simpletest.

1. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +48,18 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    $USER_AGENT = $_SERVER['HTTP_USER_AGENT'];
   

   $user_agent should be in lower case.

2. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +48,18 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    $crawlers = variable_get("vll_crawlers", unserialize(VIEWS_LAZY_LOAD_DEFAULT_CRAWLERS));
   

   The name of the variable should be prefixed with the module name. views_lazy_load_crawlers.

3. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +48,18 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    return (bool)preg_match($pattern, $USER_AGENT);
   

   $crawlers should be wrapped in a preg_quote()?

   Space after (bool)

4. +++ b/views_lazy_load.admin.inc
   @@ -0,0 +1,35 @@
   + * Form builder. Configure annotations
   + *
   + * @ingroup forms
   + * @see system_settings_form().
   

   These look like copy paste errors.

5. +++ b/views_lazy_load.admin.inc
   @@ -0,0 +1,35 @@
   +  // Get an array of node types with internal names as keys and
   +  // "Frindly Names" as values ex.
   +  // array('page' => 'Basic Page')
   

   Same here.

6. +++ b/views_lazy_load.admin.inc
   @@ -0,0 +1,35 @@
   +  $form['#submit'][] = 'views_lazy_load_admin_settings_submit';
   ...
   +function views_lazy_load_admin_settings_submit($form, $form_state){
   +  $crawlers = explode("\r\n", $form_state['values']['views_lazy_load_crawlers']);
   +  variable_set("vll_crawlers", $crawlers);
   +}
   

   This should be required, it can be handled by system settings form once we remove the serialized

7. +++ b/views_lazy_load.module
   @@ -6,6 +6,30 @@
   +define("VIEWS_LAZY_LOAD_DEFAULT_CRAWLERS", serialize(array(
   

   Do we need to serialise this? It could just be a string for me?

8. +++ b/views_lazy_load.module
   @@ -40,3 +64,29 @@ function views_html_head_alter() {
   +  $items['admin/config/views_lazy_load'] = array(
   

   Not actually sure if this needed at all?

9. +++ b/views_lazy_load.module
   @@ -40,3 +64,29 @@ function views_html_head_alter() {
   +    'page callback' => 'system_admin_menu_block_page',
   

   Wrong callback.

10. +++ b/views_lazy_load.module
    @@ -40,3 +64,29 @@ function views_html_head_alter() {
    +    'file' => 'system.admin.inc',
    +    'file path' => drupal_get_path('module', 'system')
    

    Copy paste error, should just be the views_lazy_load.admin file

Thanks @lucaschain, I have applied the patch and tested using FireFox UserAgent Switcher plugin( "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html) "), the content is still loaded using AJAX.

Hi lucaschain,
drush vget views_lazy_load_webcrawlers
views_lazy_load_webcrawlers: "Googlebot|msnbot|Rambler|Yahoo|AbachoBOT|accoona|AcioRobot|APSeek|CocoCrawler|Dumbot|FAST\-WebCrawler|GeonaBot|Gigabot|Lycos|MSRBOT|Scooter|AltaVista|IDBot|eStyle|Scrubby"

Please find a screenshot of what I saw, I thought the view should not be loaded through AJAX. may be my understanding was wrong.

@lucaschain, Sorry I uploaded wrong file, please find the one

Hi @lucaschain , Please find the details

string(173) "/Googlebot|msnbot|Rambler|Yahoo|AbachoBOT|accoona|AcioRobot|APSeek|CocoCrawler|Dumbot|FAST\-WebCrawler|GeonaBot|Gigabot|Lycos|MSRBOT|Scooter|AltaVista|IDBot|eStyle|Scrubby/i" string(72) "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)" bool(true)

Also, FYI - I'm using Views Module - 7.x-3.11

$view = new view();
$view->name = 'all_articles_lazy_load';
$view->description = '';
$view->tag = 'default';
$view->base_table = 'node';
$view->human_name = 'All Articles lazy load';
$view->core = 7;
$view->api_version = '3.0';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Master */
$handler = $view->new_display('default', 'Master', 'default');
$handler->display->display_options['title'] = 'All Articles lazy load';
$handler->display->display_options['use_ajax'] = TRUE;
$handler->display->display_options['use_more_always'] = FALSE;
$handler->display->display_options['access']['type'] = 'perm';
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'full';
$handler->display->display_options['pager']['options']['items_per_page'] = '10';
$handler->display->display_options['pager']['options']['offset'] = '0';
$handler->display->display_options['pager']['options']['id'] = '0';
$handler->display->display_options['pager']['options']['quantity'] = '9';
$handler->display->display_options['style_plugin'] = 'default';
$handler->display->display_options['row_plugin'] = 'fields';
/* Field: Content: Title */
$handler->display->display_options['fields']['title']['id'] = 'title';
$handler->display->display_options['fields']['title']['table'] = 'node';
$handler->display->display_options['fields']['title']['field'] = 'title';
$handler->display->display_options['fields']['title']['label'] = '';
$handler->display->display_options['fields']['title']['alter']['word_boundary'] = FALSE;
$handler->display->display_options['fields']['title']['alter']['ellipsis'] = FALSE;
/* Field: Content: Image */
$handler->display->display_options['fields']['field_image']['id'] = 'field_image';
$handler->display->display_options['fields']['field_image']['table'] = 'field_data_field_image';
$handler->display->display_options['fields']['field_image']['field'] = 'field_image';
$handler->display->display_options['fields']['field_image']['label'] = '';
$handler->display->display_options['fields']['field_image']['element_label_colon'] = FALSE;
$handler->display->display_options['fields']['field_image']['click_sort_column'] = 'fid';
$handler->display->display_options['fields']['field_image']['settings'] = array(
'image_style' => '',
'image_link' => '',
);
/* Field: Content: Body */
$handler->display->display_options['fields']['body']['id'] = 'body';
$handler->display->display_options['fields']['body']['table'] = 'field_data_body';
$handler->display->display_options['fields']['body']['field'] = 'body';
$handler->display->display_options['fields']['body']['label'] = '';
$handler->display->display_options['fields']['body']['element_label_colon'] = FALSE;
$handler->display->display_options['fields']['body']['type'] = 'text_summary_or_trimmed';
$handler->display->display_options['fields']['body']['settings'] = array(
'trim_length' => '600',
);
/* Sort criterion: Content: Post date */
$handler->display->display_options['sorts']['created']['id'] = 'created';
$handler->display->display_options['sorts']['created']['table'] = 'node';
$handler->display->display_options['sorts']['created']['field'] = 'created';
$handler->display->display_options['sorts']['created']['order'] = 'DESC';
/* Filter criterion: Content: Published */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'node';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['value'] = 1;
$handler->display->display_options['filters']['status']['group'] = 1;
$handler->display->display_options['filters']['status']['expose']['operator'] = FALSE;
/* Filter criterion: Content: Type */
$handler->display->display_options['filters']['type']['id'] = 'type';
$handler->display->display_options['filters']['type']['table'] = 'node';
$handler->display->display_options['filters']['type']['field'] = 'type';
$handler->display->display_options['filters']['type']['value'] = array(
'article' => 'article',
);
$handler->display->display_options['views_lazy_load_enabled'] = 1;

/* Display: Page */
$handler = $view->new_display('page', 'Page', 'page');
$handler->display->display_options['views_lazy_load_enabled'] = 1;
$handler->display->display_options['path'] = 'all-articles-lazy-load';

There is no lazy loading with the pager, when views_lazy_load is enabled we force AJAX to TRUE and therefore the pager pulls in results via AJAX.

@lucaschain, infact the below code worked for me. Thanks

if (!$this->isClientCrawler()) {
        // Lazy loading requires an AJAX view.
        $this->view->set_use_ajax(TRUE);

        // Marking the view as executed prevents the Search API from querying
        $this->view->executed = TRUE;
        // This is a bit of a hack to make sure the view is always rendered when
        // we're lazy loading the view. By setting an "empty" area handler in the
        // header the view is rendered which is later used for switching in the
        // results.
        // @TODO, in the future we could clean out all area plugins first pass?
        if (empty($this->display->default_display->options['empty'])) {
          $this->addLoadingArea();
        }

        // Add our JavaScript to the page with an array of dom_ids that can be
        // used client side to retrieve the view instance.
        $path = drupal_get_path('module', 'views_lazy_load');
        $settings[] = $this->view->dom_id;
        drupal_add_js(array('views_lazy_load' => $settings), 'setting');
        drupal_add_js($path . '/js/views-lazy-load.js', array('scope' => 'footer', 'weight' => 10));
      }
      else {
        // Ensure that ajax won't be used if a crawler is visiting the view
        $this->view->set_use_ajax(FALSE);
      }

Two issues with your previous fix,
- "Loading.." message was still shown in the page.
- was throwing JS exception, as dom_id was not found

@lucaschain, Cool Thanks.

Why did you remove one test?, Now we can have this test also, right?

+    // Make sure that our view won't be lazy under a crawler robot visit
+    // Set our agent to some robot's agent:
+    $_SERVER['HTTP_USER_AGENT'] = "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)";
+    $view = $this->getTestView(TRUE);
+    $output = $view->preview();
+    $this->assertNotContains('Loading...', $output, 'The view output does not have the lazy loading text because of the webcrawler\'s agent.');

@lucaschain, All the 6 tests are passed in my local also, Please find the interdiff patch attached, with some formatting done as per Drupal Coding standards

1. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +50,19 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +  function isClientCrawler() {
   

   Can we add some docs for this method.

2. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -47,6 +50,19 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +    // No preg_quote here because it would escape our needed pipes
   +    $pattern = "/" . $crawlers . "/i";
   
   +++ b/views_lazy_load.admin.inc
   @@ -0,0 +1,26 @@
   +  // preg_quote here prevents some regexp character in the agent strings to mess with our comparisson
   +  $crawlers = explode("\r\n", preg_quote($form_state['values']['views_lazy_load_crawlers']));
   

   Can we not preq_quote this where we actually do the check? Seems a little funny to preq_quote() before we save the value.

+++ b/views_lazy_load.module
@@ -6,6 +6,30 @@
+define("VIEWS_LAZY_LOAD_DEFAULT_CRAWLERS", implode("|", array(

Why implode these here, they could just be an array?

Thanks for all the work on this, I've made a few changes and now I think it's ready. If you would like to test again?

Changes are:

• I renamed to user_agent rather than crawler. I think excluded user agents is more accurate.
• I fixed the test, previously the unset() on the get param was after the test and the code was never going into the user agent code.
• I refactored to save the user agents as an array so that I could use preg_quote() before do the matching. I used a bit of str_replace magic to avoid escaping the pipe.
• I moved the default user agents into a method rather than a constant.

I was thinking we could simplify this feature a whole lot more by simply removing the admin form altogether and just leaving it in variable_get(), that way users can override per site from their settings file? If we did that, it could just be a regex string and we wouldn't need any converting to/from textareas etc

*EDIT* Patch against this comment was actually an interdiff against your last patch.

Two patches, full patch from #35 and then a no-admin version as a POC

I think I prefer the no admin approach, it's much simpler. Couple of points i picked up in my own patch.

1. +++ b/includes/views/views_lazy_load_plugin_display_extender.inc
   @@ -45,6 +48,18 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
   +   * ¶
   

   Trailing space to fix on commit.

2. +++ b/views_lazy_load.info
   @@ -3,7 +3,10 @@ description = "Lazy load any views content via AJAX"
   +configure = admin/config/views_lazy_load/settings
   

   This would need removing from no-admin version.

Try again.

Version number on this issue was incorrect.

+++ b/includes/views/views_lazy_load_plugin_display_extender.inc
@@ -12,28 +11,32 @@ class views_lazy_load_plugin_display_extender extends views_plugin_display_exten
+        // Ensure that AJAX won't be used if a crawler is visiting the view
+        $this->view->set_use_ajax(FALSE);

Is there a reason we need to disable ajax and not just disable the views query? The result should be displayed without doing this?

This is done because views still need to be rendered, and things that would still use ajax like pagination should not. thats why we set ajax to false

Ahh yes, that makes sense. OK, so the rest is good? Did you have a preference on just dropping the UI? Core doesn't have a UI for things like 404_fast_paths I think it might be overkill here?

We wouldn't need a new release to add more robots, it would just be configurable from $conf in your settings.php file or with a simple variable_set in an update hook.

Will look at committing later today.

Committed #41 as it was the simplest patch. @lucaschain thanks for all your efforts here, much appreciated.