540619 - Add-ons window > Updates Tab > "Show Information" pane is blank for AMO hosted add-ons

Status: VERIFIED FIXED

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

Description

[Michael Kraft [:morac]]

Attachment: HTTP Headers returned by a AMO hosted add-on.

Recently something changed with AMO hosted add-ons such that the "Show Information" pane for updated add-ons is always blank.  The "Show Information" pane shows text for most (but not all) non-AMO hosted add-ons.  The problem appears to have been caused by a change with the AMO web site itself since older versions of Firefox that used to display "Show Info" no longer do so.  It affects all versions of Firefox from 3.0.0.x up to the nightly trunk load.  

I used the Live HTTP Headers to examine what is returned by a Mozilla hosted add-on and compare it with a non-Mozilla hosted add-on that worked.  See attached files.

You can also use the Tool->Page Info menu to look at the content type for two update URLS I used:
https://addons.mozilla.org/versions/updateInfo/90149/en-US/
http://getfirebug.com/updateInfo/firebug1.5.xml

The difference I found was that the non-Mozilla hosted add-on returned a content type of "text/xml".   The Mozilla hosted add-on returned a content type of "application/xhtml+xml".  The former will display, but the later will not.  So Firefox won't show the update text if it's returned as "application/xhtml+xml".

I'm not sure if this is technically a AMO bug or a Toolkit Add-on Manager bug, but according to https://developer.mozilla.org/en/Extension_Versioning%2c_Update_and_Compatibility#Providing_Details_about_Updates a content type of "application/xhtml+xml" is what is supposed to be used for the updateInfoURL page.  AMO is correct in returning that type, but for some reason Firefox won't display the update info in the "Show Information" page.  For that reason alone I've decided to file this against the Toolkit - Add-on Manager.

Feel free to move this bug to AMO public pages if you believe it should be filed against AMO.

Comment 1

[Michael Kraft [:morac]]

Attachment: HTTP Headers returned by Firebug (not AMO hosted)

Comment 2

[Justin Scott [:fligtar]]

Confirming that version notes aren't showing up in the client. We need to fix this ASAP. If the content type is indeed the problem, was that an IT change or our change?

Comment 3

[Michael Kraft [:morac]]

I'm not sure if that's actually the cause, but it is a difference between those that work.  Like I mentioned though the actual developer pages state that a content type of "application/xhtml+xml" is valid for version notes.

I'm particularly knowledgeable about the "application/xhtml+xml" content type.  Is what's being returned valid xhtml+xml?


Also I think this broke fairly recently as I believe it was working a few weeks ago.

Comment 4

[Michael Kraft [:morac]]

Typo: I'm NOT particularly knowledgeable....

Comment 5

[Wil Clouser [:clouserw]]

I can reproduce this in minefield, but I don't think it's an AMO problem.  As stated above, AMO is doing the right thing.  I don't know of any changes on the site that would affect this.

Can someone from the platform team check this out?

Comment 6

[Justin Scott [:fligtar]]

Wil, I can reproduce this in Firefox 3 and 3.5. I don't think it's a Firefox regression.

Comment 7

[Wil Clouser [:clouserw]]

Can someone from the platform team tell me what the problem is then?  Is it the mime type?

Comment 8

[Dave Townsend [:mossop]]

This is a result of bug 528693, I obviously didn't read the solution there properly, div tags aren't supported which is why we're getting a blank display. I'm adding support for more tags to this, they should be minor enough to be backported to the branches. <br/> is at the top of my list, what else would be helpful at this point?

Comment 9

[Wil Clouser [:clouserw]]

AMO currently allows this through the purifier:

<a href title> <abbr title> <acronym title> <b> <blockquote> <code> <em> <i> <li> <ol> <strong> <ul>

and I <div> and <br> are automatically added.  Might want to check out bug 503505, although it looks like we don't support <dl>s on AMO

Comment 10

[Justin Scott [:fligtar]]

So, what can we do replace the divs today?

Comment 11

[Dave Townsend [:mossop]]

We never should have made the client as restrictive as it is, it makes this a real pain, but let's sort that out in bug 503505. For now it probably needs to be something like the following:

Run it through something to clean out all but <ol><ul><li><b><i><strong><em> tags.

For sections of the text that are outside of <ol> or <ul> surround with <p> ... </p> and replace linebreaks with </p><p>.

Comment 12

[Wil Clouser [:clouserw]]

That needs to happen today?  How high of a priority is this, someone will have to drop something if it's a blocker.

Comment 13

[Justin Scott [:fligtar]]

Firefox 3.6 is coming out tomorrow. There's going to be a lot of add-on updating and looking at release notes to see things like "now supports 3.6". Normally I would say we could do it within a few days, but because of 3.6, I think it needs to be sooner.

Comment 14

[Wil Clouser [:clouserw]]

Attachment: temporary fix for tomorrow

Here is a temporary fix for what we discussed on IRC.  It will convert:
------------------------------------------------------
Update to XML (bug 419920)

http://google.com/
<code>some code 
here</code>
<ul><li>one</li><li>two</li></ul>
<em>arr</em>

<strong>Strong! but broken</string

<ul><li>one</li><li>two</li></ul>
------------------------------------------------------

To this:
------------------------------------------------------
    <p>
    Update to XML (bug 419920)
</p><p>
</p><p>http://google.com/
</p><p>some code 

</p><p>here
</p><p>onetwo
</p><p>arr
</p><p>
</p><p>Strong! but broken            </p>
------------------------------------------------------

Comment 15

[Dave Townsend [:mossop]]

Attachment: generated html

So I can test this. I presume we still get the <html><body> stuff surrounding?

Comment 16

[Justin Scott [:fligtar]]

Comment on attachment 422644 [details] [diff] [review]
temporary fix for tomorrow

I don't have a working AMO install right now, but the code seems to do what we discussed. Once it's on preview we can test it and make sure it works with Firefox McPicky.

Comment 17

[Wil Clouser [:clouserw]]

(In reply to comment #15)
> Created an attachment (id=422646) [details]
> generated html
> 
> So I can test this. I presume we still get the <html><body> stuff surrounding?

Yep, all that is the same

Comment 18

[Wil Clouser [:clouserw]]

r60659 and calling this "fixed."  I filed bug 540968 for when the client is fixed.

Comment 19

[Dave Townsend [:mossop]]

Comment on attachment 422644 [details] [diff] [review]
temporary fix for tomorrow

Verified against Firefox, this looks good enough for now to me

Comment 20

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

Ditto; I tested in the client using Weave, ViewAbout, and Tab Sidebar.

Verified FIXED on preview.

Comment 21

[Wil Clouser [:clouserw]]

Attachment: incremental patch to fix escaping issues

We should be escaping anything that strip_tags() doesn't remove.