Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markie’s picture

Status: Active » Needs review
FileSize
10.45 KB
5.9 KB

Uploaded for your approval.. Please place the TruncateHTML.inc_.txt file into a directory named src and rename to .inc

DamienMcKenna’s picture

Does the license in http://www.pjgalbraith.com/wp-content/uploads/TruncateHTML.php.txt preclude it from being included directly?

markie’s picture

FileSize
16.69 KB

A better patch that includes the src file. (h/t. Damien)

DamienMcKenna’s picture

markie’s picture

FileSize
18.49 KB

Updated TruncateHTML class to come to Drupal Coding Standards.

DamienMcKenna’s picture

Status: Needs review » Needs work

I suspect you should revert the hellip change, and add a default value for this new setting.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
15.58 KB

This reverts some of the unrelated changes, changes back to using the original code, and adds a note to the README.txt about where the code came from.

DamienMcKenna’s picture

FileSize
15.59 KB

It'd help if I verified that the inc file wasn't missing anything.

markie’s picture

Status: Needs review » Fixed

Added a few coder details and applied to 7.x-1.x-dev build.

lolandese’s picture

Status: Fixed » Needs work

.. applied to 7.x-1.x-dev build.

A commit has not been made. See https://www.drupal.org/node/1555182/commits.

Patch does apply cleanly but some errors are reported.

martin@martin-X501A1:~/www/orange/sites/all/modules/smart_trim$ git apply -v smart_trim-n2372857-8.patch
smart_trim-n2372857-8.patch:207: trailing whitespace.
    it under the terms of the GNU General Public License, version 2, as 
smart_trim-n2372857-8.patch:208: trailing whitespace.
    published by the Free Software Foundation. 
smart_trim-n2372857-8.patch:221: trailing whitespace.
    
smart_trim-n2372857-8.patch:228: trailing whitespace.
    
smart_trim-n2372857-8.patch:230: trailing whitespace.
        
Checking patch README.txt...
Checking patch smart_trim.module...
Checking patch src/TruncateHTML.inc...
Applied patch README.txt cleanly.
Applied patch smart_trim.module cleanly.
Applied patch src/TruncateHTML.inc cleanly.
warning: squelched 29 whitespace errors
warning: 34 lines add whitespace errors.

Tested with https://www.drupal.org/project/demo_flickr node/1.
Smart trimmed with 50 characters results in the image being displayed (HTML) but none of the text. If the HTML would not have been counted, then some text should appear.
The same for 10 words.

Thanks.

markie’s picture

@lolandese - can you hit me up on IRC (teampoop) so we can talk about what you are seeing please?

lolandese’s picture

Hi Mark,

I am at work now. I prefer using the issue queue and could send you some screenshots tonight.

It should be easy to replicate with the sandbox demo of the Flickr module. After install, just upload the patched Smart Trim module archive at admin/modules/install. Also remove the <!--break--> tag at the end of node 1 and obviously apply Smart Trim on the display of the body in Teaser view with all the right settings.

Hint:
It might be that Smart Trim kicks in before the text filter gets applied. That would explain a lot.

Thanks for looking into this.

lolandese’s picture

FileSize
275.45 KB
95.31 KB

Screenshots as promised (the image title explains what ST settings are used), plus patched module to upload in the sandbox.

  • 4b1bc47 committed on 7.x-1.x
    git commit -m "Issue #2372857 by markie, DamienMcKenna: Added D7 - Trim...
maximpodorov’s picture

Priority: Normal » Major

Please revert the commit. The used library is erroneous in processing non-English texts. See strlen() and other bad things. I have broken Russian characters after this commit.

kallehauge’s picture

As @Maximpodorov said; there is no support for non english text. I have reverted the patch since it's committed to the dev branch and would highly suggest some other way to do this :)

kallehauge’s picture

Sorry, I was a bit too quick when I created the patch. This is created from the root of the module instead of the previous one in #16 :)

  • kallehauge authored b6014a5 on 7.x-1.x
    Issue #2372857 by markie, DamienMcKenna, lolandese, kallehauge: D7 -...
markie’s picture

Priority: Major » Normal

I have reverted the commit and will look into a solution that addresses multi-byte characters.

markie’s picture

@maximpodorov
@kallehauge

Can you guys try checking out the 7.x-1.x-truncate branch and test it against some of your non standard characters. I have updated strlen() to mb_strlen(). Would love to know the specifics of "other bad things" if possible..

Thanks for your help on this!

markie

maximpodorov’s picture

Where is the branch?

DamienMcKenna’s picture

@markie: How about just using drupal_strlen?

markie’s picture

@maximpodorov: you'll have to checkout via GIT. Since it's not using a standardized name the branch will not be built into a release. Use the following command:

git clone --branch 7.x-1.x-truncate maximpodorov@git.drupal.org:project/smart_trim.git

cd smart_trim

If Damien's suggestion pans out I may just commit it back to the dev build though.

@DamienMcKenna: because I had no idea it existed? Since this was a third party class, I didn't even look. I'll give that a try. Also will have to look at using the Unicode class in D8.

DamienMcKenna’s picture

@markie: If you do change the library, make sure you document it in the README.txt.

pawel_r’s picture

If somebody has need for not counting html tags, please use: https://www.drupal.org/project/field_html_trim