-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Solve word count bug in mod_feed + some code review #8230
Conversation
} | ||
|
||
$feed = ModFeedHelper::getFeed($params); | ||
$feed = ModFeedHelper::getFeed($params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you use spaces to align here! Thanks :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed 👍
I have tested this item 🔴 unsuccessfully on 61f6253
Tested unsuccessfully
|
This PR has received new commits. CC: @Ruchiranga This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
This PR has received new commits. CC: @Ruchiranga This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
@Ruchiranga Thanks for testing! Yes you're right, the RTL function wasn't working, I used a wrong variable. I also changed the label of the parameter to Can you please test again, and confirm the problems are solved? |
I have tested this item ✅ successfully on 607a440
|
I´m not sure if this breaks something on existing sites because even if the wordcount didn´t work right before Isn´t it better to fix the wordcount instead? This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
Changing from word count to character count will break peoples sites. Imagine I have it set to 10 beforemy news feed of approximately ten words excepting html bug aftermy news fe So with this change form word to character count it cannot be accepted. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
If you want to update the PR to retain the word count then this can be tested otherwise I am afraid it will have to be closed This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
Closed for the reason above This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8230. |
About this PR
This PR solves a bug with the word count parameter of mod_feed. If you load a feed with HTML tags (for example: http://community.joomla.org/blogs/community.feed?type=rss) and set the word count to 20 there are way less than 20 words visible. The two things that cause this problem:
JHtml::_('string.truncate')
counts the characters, not the words of the string<a href="http://link.nl">
are counted tooThis PR change the text of the parameter to
Character count
and I also addsstrip_tags()
for the character count.Other changes in this PR are code style and some change to make the code more readably and logic.
Test instructions