Last Comment Bug 503505 - Update info should allow more html tags through
: Update info should allow more html tags through
Status: RESOLVED FIXED
: dev-doc-complete, verified1.9.2
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Dave Townsend [:mossop]
:
: Andy McKay [:andym]
Mentors:
: 534646 (view as bug list)
Depends on:
Blocks: 540968
  Show dependency treegraph
 
Reported: 2009-07-10 07:54 PDT by Wladimir Palant
Modified: 2010-03-22 13:07 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2-fixed
.9-fixed


Attachments
patch rev 1 (2.40 KB, patch)
2010-01-20 16:09 PST, Dave Townsend [:mossop]
no flags Details | Diff | Splinter Review
screenshot (132.34 KB, image/png)
2010-01-29 15:34 PST, Dave Townsend [:mossop]
dveditz: review+
Details
patch rev 2 (2.41 KB, patch)
2010-02-25 16:45 PST, Dave Townsend [:mossop]
robert.strong.bugs: review+
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
Details | Diff | Splinter Review

Description Wladimir Palant 2009-07-10 07:54:59 PDT
I tried to show a fairly simple update info for Adblock Plus development builds - list of change descriptions with the corresponding dates. First attempt was with dictionary lists - dl/dt/dd. Result was a blank page. After consulting updateinfo.xsl I tried a plain list with two paragraphs per list item - no success again. The br tag isn't allowed either so displaying more than one line per list item is out of question. In the end I had to give up on lists altogether, would be nice to have a little more flexibility here however.
Comment 1 Dave Townsend [:mossop] 2009-12-14 09:28:23 PST
*** Bug 534646 has been marked as a duplicate of this bug. ***
Comment 2 Dave Townsend [:mossop] 2010-01-20 16:09:27 PST
Created attachment 422650 [details] [diff] [review]
patch rev 1

This is a first WIP. It removes a lot of the context based restrictions that currently exist and allow html to be formed pretty much anyway we like. Any elements not included in the list for specific actions are stripped but their contents are still included in the display. A set of formatting, block formatting and list formatting tags are allowed to pass through (with all attributes stripped). <br> and <hr> elements are passed through but their contents are ignored. <head>, <style> and <script> are completely stripped because they normally contain various text contents that aren't meant to be human readable.
Comment 3 Wladimir Palant 2010-01-20 23:22:02 PST
Comparing with http://addons.songbirdnest.com/formatting-help.html - xhtml:u is missing as allowed inline element. Also, it would be nice to support xhtml:a but that's not just a matter of templates of course, link clicks need to be handled. As to AMO, they allow xhtml:abbr/xhtml:acronym with title attribute.
Comment 4 Dave Townsend [:mossop] 2010-01-21 09:37:14 PST
<u> I can add to the list. I intentionally left <abbr> and <acronym> out since I think allowing tooltips here would probably violate our need to avoid UI spoofing. We cannot support anchors right now for technical reasonsthat will hopefully be solved in a future release of Firefox.
Comment 5 Dave Townsend [:mossop] 2010-01-29 15:34:56 PST
Created attachment 424339 [details]
screenshot

Dan, I'd like your quick approval on this. It is a screenshot showing the new range of formatting available to add-ons in the update information and I'd just like to confirm that you see no issues with what we're allowing them to do with this change.
Comment 6 Daniel Veditz [:dveditz] 2010-02-25 15:57:16 PST
Comment on attachment 424339 [details]
screenshot

r=dveditz
Comment 7 Dave Townsend [:mossop] 2010-02-25 16:45:10 PST
Created attachment 429022 [details] [diff] [review]
patch rev 2

This is the patch that implements the changes shown.
Comment 8 Dave Townsend [:mossop] 2010-02-25 16:46:26 PST
The test extension at https://people.mozilla.com/~dtownsend/testcases/bug503505/test.xpi was the one used to generate the screenshot.
Comment 9 Dave Townsend [:mossop] 2010-02-26 09:05:26 PST
Landed: http://hg.mozilla.org/mozilla-central/rev/fe2240e4d3d3
Comment 11 Dave Townsend [:mossop] 2010-03-01 09:42:43 PST
Comment on attachment 429022 [details] [diff] [review]
patch rev 2

Backporting this to the branches would really help add-on developers and the AMO team.
Comment 12 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-01 10:33:36 PST
Comment on attachment 429022 [details] [diff] [review]
patch rev 2

a=beltzner for both branches
Comment 14 Tracy Walker [:tracy] 2010-03-22 13:07:04 PDT
verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.3pre) Gecko/20100319 Namoroka/3.6.3pre

Note You need to log in before you can comment on or make changes to this bug.