YF (Yang) Reporter
	Description • 7 years ago

STR:
1. Install an older version of an add-on, check for updates to it. e.g. https://addons.mozilla.org/zh-cn/firefox/addon/hide-caption-titlebar-plus-sma/versions/?page=1#version-3.1.5.
2. In Add-ons Manger - Gear icon - View Recently Updates, check any "Show Release Notes".


Actual result:
The HTML format support has been lost in Add-ons Manger - View Recently Updates - Show Release Notes, including line breaks, bold and others.


Additional information:
Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=505e6acd9c291504700a57ddf7e88f704f65da46&tochange=52a0d2d7639717858ce6868c19a37b95e7039736

https://addons.mozilla.org/en-US/firefox/addon/hide-caption-titlebar-plus-sma/versions/3.2.2/updateinfo/ information is normal from AMO website.

	Ryan VanderMeulen [:RyanVM]
	Comment 1 • 7 years ago

Bug 918752 looks like the most likely candidate of the bunch in that range.

	Thomas Wisniewski
	Comment 2 • 7 years ago

It doesn't seem like it. I just tried the given STR on an inbound build with a revert of that patch, and got the same (unformatted) result (see attached screenshot).

	YF (Yang) Reporter
	Comment 3 • 7 years ago

(In reply to Thomas Wisniewski from comment #2)
> Created attachment 8834670 [details]
> screenshot.jpg
> 
> It doesn't seem like it. I just tried the given STR on an inbound build with
> a revert of that patch, and got the same (unformatted) result (see attached
> screenshot).

STR: Check for updates, then View Recently Updates - Show Release Notes.

	Thomas Wisniewski
	Comment 4 • 7 years ago

Ah, thanks, I see it now. It is indeed the patch from bug 918752.

It seems that AMO reads the Accept header of XMLHttpRequests, which used to be:
  text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8

It it is only:
   */*

This is what other browsers and the web standard expects, however, so we have two obvious choices:

1) change AMO to send the formatted version (rather than plaintext) when the Accept header is simply */*
2) revert the change just for XHRs made by the browser chrome (IsSystemXHR()), keeping it for web-facing content and WebExtensions.

The second option is trivial, and I've just confirmed it will work. But I'm not sure if it would be better to change AMO instead. What you think, bz?

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 7 years ago

Or:

3) Change the _fetchReleaseNotes in toolkit/mozapps/extensions/content/extensions.xml to setRequestHeader("Accept", whateverAMOWants).

I would prefer option 1 or option 3 to option 2, honestly; the only reason to do option 2 is if we think other chrome or non-webextension extension bits would be affected.

Option 3 has the benefit that we can do it entirely within Firefox, and doesn't involve figuring out the reasons for AMO's behavior.  

Dave, thoughts?

	Dave Townsend [:mossop]
	Comment 6 • 7 years ago

Given that this is broken on release I think we should go with 1 then we can fix it everywhere. Andy, can you find someone to fix this?

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 7 years ago

Oh, I missed how long ago this broke.  Then yeah, ideally fixing on the AMO side is best.

	Andrew Swan [:aswan]
	Comment 8 • 7 years ago

redirect to Stuart for the AMO work

	scolville
	Comment 9 • 7 years ago

Patch committed to mozilla/addons-server here: https://github.com/mozilla/addons-server/commit/0a675a178c85930681f8d95a022f073fd42db33b 

Github issue is: https://github.com/mozilla/addons-server/issues/4633

	scolville
	Comment 10 • 7 years ago

This was released on amo today.