608468 - Details View should include the add-on Summary text from AMO

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[custom.firefox.lady [:tawn]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101028 Firefox/4.0b8pre

Details view currently shows the "More about this add-on" information pulled from AMO, but not the add-on summary text (the text that you see in AMO search results as well as at the top of the add-on's AMO page). Seems akin to leaving the lead paragraph out of a news article.

While list view shows the add-on description (pulled from install.rdf), if it's too long to fit the single line allotted, it gets cut off, leaving no way to view the rest of the text. The "More" link seems like it should allow you to read the cut-off text, but instead it takes you to details view. IIRC, the add-on summary text on AMO defaults to the description in install.rdf, so including it in details view would also make the behavior more logical.

Reproducible: Always

Steps to Reproduce:
1. Open Add-ons Manager
2. Find an add-on with a longer description that gets cut off with ...
3. Click "More".
Actual Results:  
Details view opens showing "More about this add-on"

Expected Results:  
Details view opens showing the add-on summary preceding "More about this add-on".

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I thought it was doing that, but even in earlier versions we only display the description on the details pane. Thanks for filing this bug.

Dave, this is supported by the API, isn't it?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Also what about the description? Shouldn't it be displayed in the details view? At least that's what I see when I open the details view of a search result like:

https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/mozmill/all/4/Darwin/4.0b8pre

Do we retrieve the wrong data from the addons.sqlite?

Comment 3

[Dave Townsend [:mossop]]

We display the short summary in the list view and the long description in the details view. If neither are available then we just show the description from the rdf. We've never in the past shown both the summary and description in the same view, although AMO seem to do it now they are pretty well separated. I certainly wouldn't block on this.

Comment 4

[custom.firefox.lady [:tawn]]

(In reply to comment #3)
> We display the short summary in the list view and the long description in the
> details view.

Which is fine except when the summary doesn't fit in list view. (How much fits depends on window/monitor/resolution size.) Not saying this rfe should block 4.0, just that it would make the UI more logical and intuitive. 

blah blah blah...More
leads the user to expect that clicking 'More' enables reading the rest of that sentence, rather than just some other details. I've since found that the rest of that cut off text is available in the 'about box', but that's not really intuitive.

Comment 6

[patrickjdempsey]

It's also important to remember that authors understand the difference between the install.rdf and the "More about this add-on" section.  They *choose* to put the information they want listed within Firefox in the install.rdf.  That is a conscious choice.  The information they put in "More about this add-on" is also a conscious choice... it's the information authors want users to know *before* installing and may include all kinds of introductory information not relevant to actually using the add-on.  If we really want a "long description" vs. the "short description" then the install.rdf needs to be setup that way... or AMO needs to have a special section for that.  At the minimum, the Addons Manager should show the "short description" beside the "preview" to mirror AMO, and then have the "long description" at the bottom after the control buttons.

Comment 7

[Justin Scott [:fligtar]]

The description field of AMO was designed to always be shown underneath the summary, so that's the way authors have worded it. It looks very odd for most add-ons to only show the description on this page, like jumping into the middle of a conversation.

I really think we should show the summary above it.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This makes it so there's a inter-paragraph space (ie, the space of one line) between the short description and the full description.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1 - ready for checkin

Comment 10

[Wil Clouser [:clouserw]]

(In reply to comment #9)
> Created attachment 504903 [details] [diff] [review]
> Patch v1 - ready for checkin

I assume you've got it under control, but don't forget this is all user input and can contain all kinds of stuff.  See https://addons.allizom.org/en-US/thunderbird/api/1.5/addon/5326 for some symbols, slashes, UTF-8, etc.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/4c1dd250fef3

Please set the in-testsuite flag as appropriate?

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #10)
> I assume you've got it under control, but don't forget this is all user input
> and can contain all kinds of stuff.  See
> https://addons.allizom.org/en-US/thunderbird/api/1.5/addon/5326 for some
> symbols, slashes, UTF-8, etc.

Yep - its handled like any other data we get from AMO. As in, we don't parse it, don't modify it, and only insert it using .textContent (so its only interpreted as text, not parsed as DOM). Its not even concatenated with the full description.

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> (In reply to comment #10)
> > I assume you've got it under control, but don't forget this is all user input
> > and can contain all kinds of stuff.  See
> > https://addons.allizom.org/en-US/thunderbird/api/1.5/addon/5326 for some
> > symbols, slashes, UTF-8, etc.
> 
> Yep - its handled like any other data we get from AMO. As in, we don't parse
> it, don't modify it, and only insert it using .textContent (so its only
> interpreted as text, not parsed as DOM). Its not even concatenated with the
> full description.

Actually we do now do a little parsing of some fields (see bug 595280) but yeah we should be very safe from script or any other kinds of injection here regardless.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Grr.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110121 Firefox/4.0b10pre