Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#9576 closed defect (bug) (fixed)

Plugin Installer; Limit desciption length in search results

Reported by: DD32 Owned by:
Milestone: 2.8 Priority: normal
Severity: minor Version: 2.8
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:


I'm not sure if this should be done Server-side, or client side, But i think it'd be a good idea to limit the short description length of plugins displayed on the Search results pages.

Currently for example, if you search by tag for "content" you'll see what i mean, A few plugins which are drastically longer than the others, It makes it much harder to browse through them.. And if the user is interested from the short description, they can click through to view the full length version.


Attachments (3)

9576.patch (771 bytes) - added by hakre 7 years ago.
Div & Style
9576.1.patch (807 bytes) - added by hakre 7 years ago.
substr (0, 255) variant
9576.diff (1.1 KB) - added by dd32 7 years ago.

Download all attachments as: .zip

Change History (29)

#1 @rmccue
7 years ago

Definitely agree. Some descriptions are much too long to display.

#2 @rmccue
7 years ago

  • Cc ryanmccue@… added

#3 @Denis-de-Bernardy
7 years ago

  • Component changed from UI to WordPress.org
  • Keywords 2nd-opinion removed
  • Owner set to matt

server side if anything. there is a short description that limits you to 255 characters. if plugin search returns anything else, than it should be fixed.

#4 @DD32
7 years ago

from memory, The API returns the Long description if the short description is empty, substr($long_desc, 0, 255); could solve it all.. But some fancy '....' could be added too.

#5 @Denis-de-Bernardy
7 years ago

then again, given how long it takes for wordpress.org tickets to get closed (makes you occasionally wonder if it shouldn't be named "Oubliette"), maybe this should get fixed after all. :-P

#6 @Denis-de-Bernardy
7 years ago

  • Keywords needs-patch added

#7 @hakre
7 years ago

@DD32: substr($long_desc, 0, 255); might break HTML used in the descriptions. do the content search example.

an idea: div-ing the content, giving a maxlen, overflow: scroll. can be solved by css then.

#8 @DD32
7 years ago

The function that makes the Automatic Excerpts could be used as well, That takes care of breaking mid-tag/word i think, with balance_tags() it should work out alright..

7 years ago

Div & Style

#9 @hakre
7 years ago

  • Keywords has-patch reporter-feedback developer-feedback added; needs-patch removed

#10 @hakre
7 years ago

I think this overflow:auto stuff looks crappy. 255 chars might be way much better, wp_kses is used an might handle unclosed tags.

7 years ago

substr (0, 255) variant

#11 @hakre
7 years ago

this looks way much better. the only thing ignored here is the case that substr() might cut and utf-8 bytesequnce apart and leaving the doors open for an ijection then.

#12 @Denis-de-Bernardy
7 years ago

  • Component changed from WordPress.org to Upgrade/Install
  • Owner matt deleted

#13 @Denis-de-Bernardy
7 years ago

  • Keywords commit added; reporter-feedback developer-feedback removed

#14 @Denis-de-Bernardy
7 years ago

I don't think your worry about utf8 injections is valid. it would get crippled by the ... that get added. patch applies cleanly against today's trunk.

#15 @hakre
7 years ago

There is a type of attack based on crippled UTF-8 with a constant bytesquence following (this time '...'). If it is the case with '...' I do not properly know but generally spoken, stuff should be done the correct way. This is not the case here in terms of taking care of the encoding. infact, no care is taken about the encoding. That is what I wanted to note to the patch.

#16 @Denis-de-Bernardy
7 years ago

Well, yeah... bit if any such attack affects this patch, there arguably are scores (and I mean scores) of other areas where mb safe functions should be used instead of default php functions.

My personal hope is that php will eventually merge the mb_ functions' code into the normal string functions. I mean, heck, it's the only reasonable way to fix all of the broken php code around. The overload ini setting just isn't enough, and using wrapper functions with conditionals on every built-in string function adds lots of overhead.

#17 @hakre
7 years ago

probably, yes. as said, just for reference.

i think php 6 will have explicit binary strings next to better utf8 support in strings. see http://php.oregonstate.edu/manual/en/language.types.type-juggling.php and the cast to (binary) fyi.

just overloading standard string function with mb functions (which is possible since some more time) does not help per se and can create new fields of problems instead. see the css classnames implementaion page for more utf8 related php links.

#18 follow-up: @Denis-de-Bernardy
7 years ago

Actually, overloading currently isn't possible. I mean, it affects only a handful of functions (and it breaks *huge* amounts of scripts, precisely due to that).

#19 in reply to: ↑ 18 @hakre
7 years ago

Replying to Denis-de-Bernardy:

Actually, overloading currently isn't possible. I mean, it affects only a handful of functions (and it breaks *huge* amounts of scripts, precisely due to that).

I meant the overloading for certain string functions.

#20 @Denis-de-Bernardy
7 years ago

yaya, me too. tried it, hated it. broke everything.

#21 @ryan
7 years ago

Should we use wp_html_excerpt() here?

#22 @DD32
7 years ago

Should we use wp_html_excerpt() here?

I was thinking Not, since that strips tags. however... If we limit the descriptions length.. it doesnt really need HTML tags anyway (And thats extra complexity than just a substr())

Mind you, I dont see a real need to allow html in there now since it can be shortened. - A short Text blurb should be enough..

#23 @hakre
7 years ago

so shall it be. last patch ok.

#24 @hakre
7 years ago

related plugin area ui ticket #9743

7 years ago

#25 @dd32
7 years ago

attachment 9576.diff added

  • Based off wp_html_excerpt()
  • Strips HTML
  • Limits to 400char (multibyte safe), and adds an horizontal ellipsis(…) if its shortened
    • Doesnt use mb_strlen(), So its not entirely mb nice, any mb strings of > 200char & < 400char will end up with an ellipsis when its not needed - WordPress doesnt currently have a compat function for it.
  • Finally, re-converts multiple lines
    • And strips consecutive lines/leading/trailing lines

#26 @ryan
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11362]) Trim long descriptions in plugin installer search results. Props DD32. fixes #9576

Note: See TracTickets for help on using tickets.