436790 - "Get add-ons" should replace newlines in description with space

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: API, defect)

Description

[Gids Goldberg]

The 'get add-ons' window doesn't do a good job handling <br> tags. It appears to strip them out, which can lead to words running together in the add-on description. I found this issue with a recommended (at the time of writing) add-on 'Taboo'

Steps to reproduce:
-------------------
Tools > Add-ons > Get addons
Search for 'taboo'

Actual result
-------------
Description begins:

'The cure for tabitisIf you keep tons of tabs open'

Expected Result
---------------
<br> tag from description (https://addons.mozilla.org/en-US/firefox/addon/5756) is replaced with a space or other character which prevents the words running together.

Comment 1

[Dave Garrett]

Changing to AMO API, as that may be where this can be fixed.  

Comment 2

[Kohei Yoshino [:kohei]]

The API removes linebreaks from the summary, description and eula. They should be kept or should be replaced with white spaces.

https://services.addons.mozilla.org/en-US/firefox/api/1.1/addon/5756

Comment 3

[Kohei Yoshino [:kohei]]

Attachment: patch

This patch keeps linebreaks as-is in the API.

* These linebreaks will look like spaces on the Add-ons Manager.
* This allows API users to replace these linebreaks with <br /> themselves.

Comment 4

[Kohei Yoshino [:kohei]]

Attachment: patch

oops

Comment 5

[Wil Clouser [:clouserw]]

The patch doesn't change anything when I test it

Comment 6

[Dave Garrett]

Comment on attachment 372062 [details] [diff] [review]
patch

I know nothing about the API, but just by looking at this I ask: Shouldn't $sanitize_replacements get " " as a replacement for something instead of "", an empty string?

Comment 7

[Fred Wenzel [:wenzel]]

From the bug title, I was thinking the same thing. It's probably a typo.

Comment 8

[Kohei Yoshino [:kohei]]

Tested again and found that these linebreaks *didn't* look like spaces on the Add-ons Manager. Seems like I made an error.

So the $sanitize_replacements should replace \cJ (\n) and \cM (\r) with a space + a linebreak. Just replacing with a space will resolve the initially reported problem (summaries on the Get Add-ons window), but there are still problems:

1) The EULA dialog on the Add-ons Window. Search Aero Fox and hit Add to Firefox button to see the dialog. Linebreaks are stripped by API.

https://addons.mozilla.org/ja/firefox/addons/policy/0/6070/39291
https://services.addons.mozilla.org/en-US/firefox/api/1.1/addon/6070

2) Sites/services which use the API. We (Mozilla Japan) use the API for addons.mozilla.jp and have the problem. Please see the example of Minimap Sidebar:

https://addons.mozilla.org/en-US/firefox/addon/5203
https://services.addons.mozilla.org/en-US/firefox/api/1.1/addon/5203
https://addons.mozilla.jp/firefox/details/5203

I mean, linebreaks should be left as-is...

Comment 9

[Kohei Yoshino [:kohei]]

Attachment: patch: convert linebreak into space+linebreak

Comment 10

[Wil Clouser [:clouserw]]

This appears to already be fixed (e.g. https://preview.addons.mozilla.org/en-US/firefox/api/1.1/addon/5203 ).  Only I didn't do anything.

It turns out services.addons.mozilla.org is over 5000 revisions behind what is in trunk right now and a lot of this code has changed.  It all needs to be reviewed though (why haven't people been complaining about features that don't show up?) and that's going to take some time.

Comment 11

[Kohei Yoshino [:kohei]]

Understood. We'll wait for the deployment of the changes...