Useless replacement for some alias glyphs. #685

Merged
merged 5 commits into from Jan 3, 2012

8 participants

@brianteeman
Joomla! member

The code is not just useless it is confusing and illogical.
The proposed patch removes the replacement for < > which get stripped from titles anyway
The proposed patch removes the illogical and non-translateble replacement of " with q

The only issue is that it replaces an & with a -. It would be better to just strip it completely or you end up with double - in the alias

Eg Cat & Dog becomes cat--dog where cat-dog is better

Personally I would prefer the replacement to be &=and or whatever is the appropriate translation but as time is short before 2.5 I'm happy that the & is stripped

@infograf768
Joomla! member

To get rid of the double hyphen, just need to add
// Delete double hyphens
$str = str_replace('--', '-', $str);

before
return $str;

We can't use translations here as it would only work for pure ASCII languages.

Sorry, I do not know how to modify this pull...

@brianteeman
Joomla! member

Thats good enough to make me happy ;)

@joomla-jenkins

Unit testing complete. There were 1 failures and 0 errors from 1971 tests and 11142 assertions.
Checkstyle analysis reported 199 warnings and 0 errors.

@ianmacl

So how do these characters get dealt with in this method after the change? They are reserved characters and should need to be dealt with somehow. It seems that this change would leave these characters in. Is there something that I'm missing?

@jonnsl

this wouldn't break the old URL, when a article is re saved?

@brianteeman
Joomla! member

No because an alias is only created when the alias is field is blank

@AmyStephen

@ianmacl

The str_replace two lines down filters the alias to ensure alphanumeric values and therefore will replace those values with dashes absent that line.

https://github.com/infograf768/joomla-platform/blob/97fac72366a5bbe88b47a24c63603a2058c78ff7/libraries/joomla/filter/output.php#L98

Since it is possible to use a OnSave plugin to substitute values, and since the replacements are in no way universal or standard, I also question it's existence in core.

@infograf768
Joomla! member

We need to add in this patch the code I added above in case of double hyphens.
Can someone do that?
In fact, this patch reverts the ASCII alias making to the 1.5 behavior, which was changed for no reason before 1.6 beta.
The special characters are dealt with OK, as it was in 1.5.

@infograf768
Joomla! member

In fact, to be on the safe side, we could also take care of the < and > in both the ascii function and the unicode function, and change them to spaces which are taken care of later on.

@infograf768
Joomla! member

Added some changes to prevent double hyphen and changing > and < to spaces in the ascii as well as the unicode variant.

@joomla-jenkins

Build triggered by changes to the head.

Unit testing complete. There were 1 failures and 0 errors from 1971 tests and 11142 assertions.
Checkstyle analysis reported 199 warnings and 3 errors.

@pasamio

Why isn't the quote being removed any more?

@infograf768
Joomla! member

The double quote is removed by
// Remove any duplicate whitespace, and ensure all characters are alphanumeric
$str = preg_replace(array('/\s+/', '/[^A-Za-z0-9-]/'), array('-', ''), $str);

Example:
"Administrator's & other stuff's"
gives as alias
administrators-other-stuffs

But, as
"Administrator's "& other stuff's"
gives
administrators--other-stuffs (double hyphen)
I have now added it to the array.

@joomla-jenkins

Build triggered by changes to the head.

Unit testing complete. There were 1 failures and 0 errors from 1971 tests and 11142 assertions.
Checkstyle analysis reported 199 warnings and 3 errors.

@infograf768
Joomla! member

no idea where the failure comes from...

@chdemko chdemko commented on an outdated diff Jan 2, 2012
libraries/joomla/filter/output.php
@@ -90,15 +90,16 @@ public static function stringURLSafe($string)
$lang = JFactory::getLanguage();
$str = $lang->transliterate($str);
+
@chdemko
chdemko added a note Jan 2, 2012

Checkstyle: Whitespace found at end of line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chdemko chdemko commented on an outdated diff Jan 2, 2012
libraries/joomla/filter/output.php
// Remove any duplicate whitespace, and ensure all characters are alphanumeric
$str = preg_replace(array('/\s+/', '/[^A-Za-z0-9\-]/'), array('-', ''), $str);
+
@chdemko
chdemko added a note Jan 2, 2012

Checkstyle:

  • Whitespace found at end of line
  • Functions and classes must not contain multiple empty lines in a row; found 2 empty lines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@infograf768
Joomla! member

This should now fix it.

@joomla-jenkins

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11145 assertions.
Checkstyle analysis reported 199 warnings and 2 errors.

@chdemko chdemko merged commit 64520f4 into joomla:staging Jan 3, 2012
@pasamio

Err, this was committed with checkstyle errors?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment