Abstract:
The ping.module requires a site slogan to work. This shouldn't be, since a site slogan is not mandatory.

Details:
The ping.modules fires a ping only if the following condition is met (function ping_cron()):

if (variable_get('site_name', 0) && variable_get('site_slogan', 0)) {
  ... Check for update and ping ..
}

No ping is done, if either site_name or site_slogan is empty.

Solution:
I suggest to introduce a function _ping_get_full_site_name() like this:

function _ping_get_full_site_name() {
  $ret = variable_get('site_name', '');
  if (!empty($ret) && variable_get('site_slogan', 0)) {
    $ret .= ' - ' . variable_get('site_slogan', '');
  }
  return $ret;
}

And use it in ping_cron():

function ping_cron() {
  global $base_url;
  $sitename = _ping_get_full_site_name();
  if (!empty($sitename)) {
    if (db_num_rows(db_query("SELECT nid FROM {node} WHERE status = 1 AND moderate = 0 AND (created > '". variable_get('ping_cron_last', time()) ."' OR changed > '". variable_get('ping_cron_last', time()) ."')"), 1)) {
      _ping_notify($sitename, $base_url);
    }
  }
  variable_set('ping_cron_last', time());
}
CommentFileSizeAuthor
#17 ping-no-slogan.patch_3.txt873 bytesbreyten
#15 ping-no-slogan.patch_2.txt1021 bytesbreyten
#12 ping-no-slogan.patch_1.txt907 bytesbreyten
#10 ping-no-slogan.patch_0.txt1.04 KBbreyten
#8 ping-no-slogan.patch.txt1.04 KBbreyten
#1 ping-slogan.patch1.07 KBbreyten

Comments

breyten’s picture

breyten commented
Version: 4.6.3 » x.y.z
Status: Active » Needs review
StatusFileSize
new ping-slogan.patch1.07 KB

patch attached that removes the need for the slogan, by including it only optionally.

breyten’s picture

breyten commented

To clarify a little, I did not do what the original poster suggested, but just added a few lines.

breyten’s picture

breyten commented
Assigned: Unassigned » breyten
Status: Needs review » Reviewed & tested by the community

Still applies, updating status since this is a simple patch (and hopefully it will get on the radar of some more people).

dries’s picture

dries commented
Status: Reviewed & tested by the community » Closed (works as designed)

That's by design, and documented on the drupal-setting page.

gerd riesselmann’s picture

gerd riesselmann commented

Hmm, where exactly is this documentated? The drupal-settings page (if this is www.example.org/admin/settings) only states that

The slogan of this website. Some themes display a slogan when available.

There is no setting page for the ping.module, at least not for version 4.6, and the behavior also isn't mentioned in the handbooks, neither within section "Configuration and customization - Configuration - Settings", nor within the documentation of the ping.module.

dries’s picture

dries commented

See the form description at 'admin/settings/drupal'.

breyten’s picture

breyten commented

Dries, this isn't about pinging drupal.org (or any other drupal site that has a Druapl Directory), but this is about sites pinging ping-o-matic.com (and technorati.com). Would they still need to require a site slogan for that sort of pinging? If so, why? It doesn't seem to make sense to me.

breyten’s picture

breyten commented
Title: No pinging if no site slogan » Don't require site slogan for sending pings
Status: Closed (works as designed) » Needs review
StatusFileSize
new ping-no-slogan.patch.txt1.04 KB

Changed the title to reflect the issue better, and updated the acompanying patch.

drumm’s picture

drumm
he/him
Location NY, US
commented

+1 Code looks okay to me. (leaving for Dries to decide on comitting)

breyten’s picture

breyten commented
StatusFileSize
new ping-no-slogan.patch_0.txt1.04 KB

Keeping up with CVS...

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks! :)

breyten’s picture

breyten commented
Priority: Normal » Critical
StatusFileSize
new ping-no-slogan.patch_1.txt907 bytes

Something definitely went wrong when applying the patch though. Currently, it's broken. New patch attached.

breyten’s picture

breyten commented
Status: Fixed » Needs review

Uh, forgot to update status.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

Lets keep the site slogan default consistent, use '' instead of 0. Since we are checking to see if there are characters in a string, it is best to be explicit and say strlen(...) > 0 in if statments.

breyten’s picture

breyten commented
Status: Needs work » Needs review
StatusFileSize
new ping-no-slogan.patch_2.txt1021 bytes

That's a good idea. using that for the title too now.

dries’s picture

dries commented
Status: Needs review » Needs work

Let's not append the slogan. That part was removed intentionally.

breyten’s picture

breyten commented
Status: Needs work » Needs review
StatusFileSize
new ping-no-slogan.patch_3.txt873 bytes

Ahhh :) Well that simplifies the patch a lot :)

dries’s picture

dries commented

AFAIK, we never use strlen to check if a value (variable_get) is set ... maybe this is a valid case, but please double-check this. Thanks.

breyten’s picture

breyten commented

Double check. But the check is actually superfluous, since the default site title is 'drupal' and the settings page doesn't allow you to set an empty title (albeit one with spaces though).

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

I like using strlen() since it makes the fact that it is a string explicit.

BUt thats beside the point since the default value is either 'Drupal' or 'drupal' everywhere else, and should be this here. For example, I save that form without touching the name for plenty of test sites and such a test site would evaluate this test to true. It should be variable_get(...) != 'drupal'.

A separate patch can make the case of all these 'Drupal's and 'drupal's consistent if anyone is motivated enough to do it.

dries’s picture

dries commented
Status: Needs work » Closed (fixed)

AFAIK, this has been fixed by another issue.