Username replacement with @ doesn't work in start

entr3p - June 13, 2009 - 06:06
Project:Facebook-style Statuses (Microblog)
Version:6.x-2.x-dev
Component:Code - Functionality
Category:bug report
Priority:normal
Assigned:IceCreamYou
Status:closed
Description

I have everything set up and if let's say I type in "Hello @joe. How's it going?" the "@joe" works fine and links to the profile. Thing is, if I have a sentence like "@joe what do you like?" It doesn't replace the "@joe" with a link..even though in the "Mentions" view they both show up because I have @joe.

#1

IceCreamYou - June 14, 2009 - 03:16
Status:active» postponed (maintainer needs more info)

Hmm, are you sure you spelled "joe" correctly? Capitalization counts... I can't reproduce this problem and the code that handles the replacement is very simple and looks quite accurate. The "@" sign at the beginning of a status is explicitly special-cased, so this really shouldn't be a problem.

To see for yourself, search for "$pattern" in your facebook_status_tags.module file. The first result is the regex pattern used in the link replacement when you view a status.

//$op is a parameter of the function and is, in this case, "@."
'/([\s]'. $op .'.+\b)|(^'. $op .'.+\b)/U'

//With $op replaced, the Regex pattern will look like this:
/([\s]@.+\b)|(^@.+\b)/U

//In English, that reads as
//"find a string of text that starts with any whitespace character followed by the 'at' symbol followed by non-whitespace characters followed by a word break,
//OR look at the beginning of the entire text for an 'at' symbol followed by non-whitespace characters followed by a word break."

Exactly the same pattern is used later in the code to save the reference that would make the status show up in the "mentions" view. So if it works in one place, it should work in both.

My advice for you right now is to clear your site caches and browser cache and try again. Make sure you have the latest development copy of the module without any extraneous files left over from previous versions.

#2

entr3p - June 15, 2009 - 03:28

yup I'm sure. I will show true and tested message here:

hmmm @entr3p <-- Provides Link

@entr3p how's the testing going? Everything seems buggy to me.. <-- Doesn't provide link

I put the boldness to show that the part of that text is a link.

#3

entr3p - June 15, 2009 - 03:34

Oh and same thing happens in the "mentions" as well.

Here is an image screenshot.

http://i42.tinypic.com/209svsz.png

#4

IceCreamYou - June 15, 2009 - 04:18

Well, it's something weird with your system. As you can see it works in standard conditions although I know that doesn't help you. You may have an unusual regex configuration or something. Like I said before, reinstall FBSS (completely delete the facebook_status folder when you do so), clear your site and browser caches, and try again. This is very likely not an issue with the module so I'm forced to mark as "won't fix" unless this can be reproduced.

#5

entr3p - June 16, 2009 - 02:44

Okay. Good thing this is a test site as I can reinstall everything from scratch. I'll let you know what happens then but thank you for helping. =)

#6

IceCreamYou - June 19, 2009 - 03:58
Status:postponed (maintainer needs more info)» won't fix

Okay, I hope it works. If it doesn't, and you get enough information about this that I would have something to go on to figure out why this happened for you, feel free to reopen the issue.

#7

Deciphered - July 21, 2009 - 05:42
Status:won't fix» active

Issue confirmed and fix is below:

Change line #342 of facebook_status_tags.module from:

  $pattern = '/([\s]'. $op .'.+\b)|(^'. $op .'.+\b)/U';

To:

  $pattern = '/(\B'. $op .'.+\b)/U';

#8

IceCreamYou - July 29, 2009 - 20:06

That looks reasonable but I haven't had a chance to test it. I would like to know why there's any difference as well, as the current version does seem to work for most people... but I'll see about committing the change this weekend.

#9

IceCreamYou - August 3, 2009 - 18:16
Status:active» won't fix

#7 is incorrect. \B is a negated word boundary, so the proposed pattern would match:

@username
@username text
text @username
text @username text
text@username
text@username text

This will match things like email addresses (me@aol.com) which we don't want. The original pattern matches:

@username
@username text
text @username
text @username text

...which is correct.

#10

Deciphered - August 3, 2009 - 22:12
Status:won't fix» active

@IceCreamYou

While I don't doubt you're correct in saying that the code I supplied isn't the exact solution, it doesn't mean that you shouldn't attempt to find the correct solution.

It's likely that different server environments are the cause of the issue, so while you can't replicate the issue doesn't mean the issue doesn't exist.
I can personally reproduce the issue on Windows XP using XAMPP.

Cheers,
Deciphered.

#11

IceCreamYou - August 4, 2009 - 05:48
Status:active» postponed (maintainer needs more info)

Try this pattern:

%(\A@\w+\b)|((?<=\s)@\w+\b)%U

The pattern in the module does what it's supposed to per the PCRE spec and PHP docs on PCRE. If someone can find a pattern that works in more environments, I will use it; however, if not, I'm not going to worry about this issue, hence the "won't fix" status. This is partially because this could easily be a bug server-side, partially because I can't test different server environments (especially if I don't even know which environment I'm supposed to be testing--we're talking PHP version and the plugins it was compiled with, not OS) and partially because I have more important things to worry about.

There is a remote possibility that the problem is not with the Regex pattern but instead with the preg_match_all() patterns returned via the referenced $matches parameter. However, this seems either extremely unlikely or a server-end bug, because I can't find such behavior documented anywhere. If this is in fact the case, it may be that some servers are configured to use different default behavior for the sorting parameter; try adding PREG_PATTERN_ORDER as the fourth parameter of preg_match_all() in facebook_status_tags.module on line 343 to test this.

If no one can get this to work, I will consider adding a PHP string-based alternative. I've already written the basic logic; you can test it using the code below (put it in the body of a page).

<pre><?php
$options
= array(
 
'@test',
 
'@test text',
 
'text @test',
 
'text @test text',
 
'text@test',
 
'text@test text',
 
'@test@helper',
 
'@test @helper',
 
'@test text @helper',
 
'text @test @helper',
 
'@test @helper text',
 
'@test text @helper text',
 
'text @test text @helper',
 
'text @test text @helper text',
 
'text@test @helper',
 
'text@test text @helper',
 
'text@test text@helper',
 
'text@test @helper text',
 
'text@test text@helper text',
);
foreach (
$options as $option) {
 
$matches = array();
 
_facebook_status_tags_filter_extra($option, $matches);
  echo
"\n\n". $option .': '. implode(', ', $matches);
}
//In the module, instead of modifying $matches, this would work the same as _facebook_status_tags_filter().
function _facebook_status_tags_filter_extra($subject, &$matches, $op = '@') {
 
$words = str_word_count($subject, 2, $op);
  foreach (
$words as $pos => $word) {
    if (
drupal_substr($word, 0, 1) == $op && strpos($word, $op, 1) === FALSE) {
     
$matches[] = drupal_substr($word, 1);
    }
  }
}
?>
</pre>

#12

IceCreamYou - August 6, 2009 - 05:44
Assigned to:Anonymous» IceCreamYou
Status:postponed (maintainer needs more info)» needs review

It turns out the string-based method is actually about 15% faster than the Regex-based method anyway, so I'm going to switch to that if I can get someone to confirm that it works. Here's the function that will replace _facebook_status_tags_filter():

<?php
function _facebook_status_tags_filter($subject) {
 
$words = str_word_count($subject, 2, '@#');
 
$search = array();
 
$replace = array();
  foreach (
$words as $pos => $word) {
   
$op = drupal_substr($word, 0, 1);
   
$match = drupal_substr($word, 1);
    if ((
$op == '@' || $op == '#') && strpos($word, '@', 1) === FALSE && strpos($word, '#', 1) === FALSE) {
      if (
$op == '@') {
       
$account = user_load(array('name' => $match));
        if (
$account->uid) {
         
$link = $op . theme('username', $account);
        }
      }
      else {
       
$term = _facebook_status_tags_get_term($match);
        if (!empty(
$term)) {
         
$dest = _facebook_status_tags_resolve($term);
          if (
$dest) {
           
$link = l($term->name, $dest);
          }
        }
      }
      if (
$link) {
       
$search[] = $word;
       
$replace[] = $link;
      }
    }
  }
  return
str_replace($search, $replace, $subject);
}
?>

#13

IceCreamYou - August 6, 2009 - 06:56
Status:needs review» fixed

I'm actually quite happy with this - not only should it solve this mystery issue, and not only is it faster, but it's also a much cleaner approach. (Fix is in the next dev.) Cheers!

#14

System Message - August 20, 2009 - 07:00
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

#15

jakewalk - September 16, 2009 - 09:38

The filter function doesn't recognize usernames, that have a whitespace in it. Any ideas for a fix?

#16

IceCreamYou - September 16, 2009 - 21:40

No. That won't be possible until 3.x, and maybe not even then. There's just no reasonable way to do it without an enormous hit on processing time.

#17

Deciphered - September 17, 2009 - 04:56

IceCreamYou,

The only logical way to solve that issue is to use a start (@) and end point to capture the username, such as [@username], @username@, etc. While it's not as nice to look at, it is really the only viable solution. Making it optional/configurable is also a good option.

#18

IceCreamYou - September 17, 2009 - 05:12

Using a string-based method to do that would--like I said--take up too much processing time. This could be relatively easily done with regex but I have too little time to deal with the obscure differences that seems to cause.

Besides, that is a completely separate issue and doesn't belong in this thread.

#19

IceCreamYou - September 29, 2009 - 23:47

@jakewalk, it randomly occurred to me just now that you could use Deciphered's module Mentions to do what you want, although you'll have to choose a different character than the "at" symbol (@) for the referencing if you still want to use FBSST.

#20

jakewalk - October 15, 2009 - 19:45

We "solved" the problem through allowing only usernames that don't contain a whitespace. But thanks for your help anyway.

#21

IceCreamYou - October 26, 2009 - 06:18

FYI, I just switched back to Regex, albeit with a much better pattern. Anyone who had problems with Regex before should try the latest dev (after it gets generated) to make sure it works for you.

On the other hand, if it doesn't work for you, it is unlikely that I will fix it. I have read extensively on the subject now and the pattern does what it should according to all the standard PHP/PCRE distributions of which I'm aware. I will not offer a string-based method either because it is too crippled in comparison.

 
 

Drupal is a registered trademark of Dries Buytaert.