Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I tried to select the length as something other than 600 characters... but it won't let me.
Comments
Comment #1
droplet CreditAttribution: droplet commentedComment #2
droplet CreditAttribution: droplet commentedsame patch work for both D7 & D8.
Comment #3
njardim72 CreditAttribution: njardim72 commentedhi,
patch 1045786.patch only works around the issue of being able to select the length of trimmed description. it doesn't actually trim the content
thanks,
njardim
Comment #4
droplet CreditAttribution: droplet commentedahh I can see what you means. it maybe by design, looking for more comment.
if we need to add it in, then may need to add a new filter for aggregator too.
Comment #5
njardim72 CreditAttribution: njardim72 commentedhi droplet,
we really need to trim the feeds contents, some feeds are just to long for displaying
thanks,
njardim
Comment #6
njardim72 CreditAttribution: njardim72 commentedmaybe this can help:
http://api.drupal.org/api/drupal/includes--unicode.inc/function/truncate...
just need to know where to call this from to display the trimmed feeds
regards,
njardim
Comment #7
droplet CreditAttribution: droplet commentedas I can tell, text_summary is the best for trim contents. (safety for HTML tags)
Comment #8
njardim72 CreditAttribution: njardim72 commentedany news regarding this issue?
thanks,
njardim
Comment #9
Désiré CreditAttribution: Désiré commentedtest for the aggregator settings page
Comment #10
xjmThanks for the patches! Reading this test, it's not clear to me that it will fail without the bug fix you provide in #9. Does the test fail locally when you do not apply the fix? If it does fail locally, could you upload the test in a patch by itself to demonstrate that it fails without the bugfix?
Also, here are a couple minor notes about the code documentation:
I'd change this to: "Tests the settings form to ensure the correct default values are used."
We need a documentation block for this class. See http://drupal.org/node/1354#classes for more information.
Finally, note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.
Comment #11
Désiré CreditAttribution: Désiré commentedHere are the rerolled and updated patches.
a test only patch too, btw the original test only patch is here: http://drupal.org/node/1324102#comment-5183936, tihs is the same patch, but the issue is a duplication of this.
Comment #12
Désiré CreditAttribution: Désiré commentedComment #13
Désiré CreditAttribution: Désiré commentedoh, sorry, wrong patch...
(test only was correct)
Comment #14
xjmThis works too, but it should be "Tests..." :) Other than that, this looks good to me. Waiting to see what testbot says.
Comment #15
Désiré CreditAttribution: Désiré commentedoh, my... :)
Comment #17
xjmNote that there is a testbot issue at the moment, which is why the patch failed. We'll want to re-test it once that is fixed.
Comment #18
Désiré CreditAttribution: Désiré commentedComment #19
xjmAlright, the revised patch in #15 looks ready to me.
Comment #20
catchCommitted/pushed to 8.x, thanks!
Looks like this should be backported to 7.x.
Comment #21
Désiré CreditAttribution: Désiré commentedBackported to D7
Comment #22
twistor CreditAttribution: twistor commentedTests pass, looks good.
Comment #23
sunIn case of a form validation error, the drupalPost() ends up on the same page on which the original form is shown, containing the values that were attempted to be submitted ($edit), and settings have not been saved yet.
Thus, there's a chance that the assertFieldByName() assertions will wrongly succeed. You may overcome this by asserting that the "The configuration has been saved." message appears on the page after the drupalPost().
Comment #24
aspilicious CreditAttribution: aspilicious commentedSun, just to be clear do you think we have to add something like
first in D8 or do you prefer it to be included in the D7 patch.
Comment #25
xjmSo do we need a followup patch/NW for D8 to correct the test?
Comment #26
xjmAlright, sun suggested I "be bold," so here it is.
Let's create a followup patch for D8, a new patch rolled against current HEAD. Something like the code @aspilicious suggests should work fine; just add it right after the drupalPost().
Then, create a D7 backport patch that includes both the original patch and that addition. You can name the D7 patch
my_patch_name-d7.patch
to keep the testbot from testing it while the issue is filed against D8.Tagging novice for the above. :) Thanks in advance!
Comment #27
Albert Volkman CreditAttribution: Albert Volkman commentedD8 patch updated per xjm's request. D7 patch to follow.
My first *possible* core patch. Excited!
Comment #28
Albert Volkman CreditAttribution: Albert Volkman commentedComment #29
aspilicious CreditAttribution: aspilicious commentedThe last part needs to be removed.
the // and everything behind it.
-10 days to next Drupal core point release.
Comment #30
Albert Volkman CreditAttribution: Albert Volkman commentedAh, got too excited. Fixed!
Comment #31
Désiré CreditAttribution: Désiré commentedasserText needs a translated string,(see: http://drupal.org/node/265828)
so the correct addition is:
$this->assertText(t('The configuration options have been saved.'));
Comment #32
aspilicious CreditAttribution: aspilicious commentedYeah... I did a grep through core and most of the assert text messages are packed in a t() function. So yeah..
Comment #33
Albert Volkman CreditAttribution: Albert Volkman commentedThanks for the catch Désiré! Updated D8 module and backported D7 attached.
Comment #34
xjmLooks good to me. Thanks @Albert Volkman!
Comment #35
catchCommitted/pushed to 8.x, back to D7 again.
Comment #36
xjmD7 patch available in #33.
Comment #37
webchickRe-uploading the patch in #33 without the -D7 at the end so testbot tests it.
Comment #38
webchickCommitted and pushed to 7.x. Thanks!
Comment #40
zilla CreditAttribution: zilla commentedis there a link to the entire updated aggregator core module that i might simply replace versus applying this patch? i'm not sure how to apply this patch, but this is precisely the issue i've been encountering (trimmed length)
Comment #41
Albert Volkman CreditAttribution: Albert Volkman commentedThis fix will be included in the next release of Drupal. Short of waiting until then, your only option currently is to use the supplied patch.
Comment #42
zilla CreditAttribution: zilla commentedthanks. just used text mate to edit that one line and committed. oddly, when i save to trimmed length 200, it saves on admin, but when i view actual aggregator category with all items, it's still showing long entries...