444817 - Limit size of add-on desciptions in feature element

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Michael Morgan [:morgamic]]

Truncate the summary of summaries in the feature element so they can't take up too much space.  If people want to have verbose summaries, give them the chop.

Comment 1

[Dave Garrett]

Isn't this bug 443977 or are you referring to another field?  (not entirely sure what you mean by "summary of summaries in the feature element")

Comment 2

[Fred Wenzel [:wenzel]]

Well, I think lorchard accidentally added a patch to bug 443977 that should have ended up here.

Comment 3

[Les Orchard [:lorchard]]

Attachment: quick feature element patch to truncate too-long descriptions

Yup, this patch was posted to a different bug, since we couldn't find this one earlier yesterday.  Posting the patch to this bug now for your perusal

Comment 4

[Dave Garrett]

Comment on attachment 330784 [details] [diff] [review]
quick feature element patch to truncate too-long descriptions

As per bug 443977 comment #4, the limit should be 250 characters, not the 400 I gave as an initial suggestion in the description.

Comment 5

[Les Orchard [:lorchard]]

Attachment: changed limit from 400 to 250

Okay, posted a patch with a limit of 250.

Comment 6

[Dave Garrett]

This patch has been sitting here for a while now. (and it's listed as severity "blocker", at that ;) Is this going to get used or should something else be done?

If the limit is correctly enforced (bug 443977) and any over the limit are truncated then this patch would be redundant.

Comment 7

[Les Orchard [:lorchard]]

Comment on attachment 330788 [details] [diff] [review]
changed limit from 400 to 250

Adding an r? for the patch, which appears to still work 3 months or so later

Comment 8

[Ryan Doherty (:rdoherty)]

Comment on attachment 330788 [details] [diff] [review]
changed limit from 400 to 250

Looks good works well.

Comment 9

[Les Orchard [:lorchard]]

Belatedly checked in as r20012

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot showing regression

For "Clupedia Toolbar" in the screenshot, we simply show the ellipsis, but, according to the patch comments, we should display the last full word up to the 250-character limit and then ellipsize (look Ma, new word!)

Comment 11

[Fred Wenzel [:wenzel]]

(In reply to comment #9)
> Belatedly checked in as r20012

ZOMG we are in the 20ks!

Comment 12

[Dave Garrett]

(In reply to comment #10)
> 250-character limit and then ellipsize (look Ma, new word!)

Look Ma, new word... and I misspelled it 2 lines after I learned it! :p

Comment 13

[Les Orchard [:lorchard]]

Crud - this works on my laptop.  Wondering if the multibyte string functions aren't working on preview.  If not, that's annoying, and there might not be a way I know to do this for non-English languages.  Trying a checkin using plain old string functions to see if it works for English on preview at least.

Comment 14

[Fred Wenzel [:wenzel]]

They have to work: We use multibyte string functions elsewhere already.

Comment 15

[Les Orchard [:lorchard]]

Huh, well, I changed all the mb_* string functions to non-mb in r20044 and it looks like things magically started working on preview.

Wondering if anything showed up in the error log, since it just looked like it was silently failing to do proper truncation.

Comment 16

[Fred Wenzel [:wenzel]]

But that's not exactly the right way to fix this if it's going to break multi-byte strings. If preview doesn't have the right packages, we need to fix preview, not un-internationalize our code.

Comment 17

[Wil Clouser [:clouserw]]

oremj says php-mbstring is installed on preview.amo.  And really, if the mb_ functions aren't working I think we'd see a lot more break.

Comment 18

[Michael Morgan [:morgamic]]

Wil will help find an all-japanese add-on to test the mb functions and help debug.

Comment 19

[Les Orchard [:lorchard]]

As it turns out, the functions are installed - a test I added in r20052 proves
that out.  

The problem ended up being that there's been a parameter change between PHP
5.1.6 and 5.2.0 - in 5.1.6, mb_strrpos() is missing support for an offset
parameter supported by strrpos() and other string functions.  I just happened
to use that one, but couldn't see the error.

Checked in a fix in r20065, hopefully be on preview soon

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED, with the following noted:

When BlogRovR (https://preview.addons.mozilla.org/en-US/firefox/addon/4689
) is featured in the "Recommended" list (displayed text follows),

"You don't have time to read ALL those Blogs! Let RovR do it for you. RovR fetches posts from your favorite blogs about anything you're browsing, and shows you summaries you can open read posts without leaving the web page you were" (...) it's 230 characters, rather than the 250 -- Les can explain it better than I, but we're splitting on "spaces, not newlines."