437158 - Update add-on install links and service

Status: VERIFIED FIXED

(www.mozilla.org :: General, defect)

Description

[Justin Scott [:fligtar]]

See bug 433408 comment 14

The new Get Personal page needs to have the Install Now links use an install trigger via the AMO install service. This requires bug 389037 to be fixed first.

Comment 1

[Justin Scott [:fligtar]]

We decided to go ahead and link to static versions and when bug 389037 is fixed, we'll switch. I'll attach the patches shortly.

Note that LinkedIn Toolbar does not yet have a Firefox 3 compatible version on AMO, so we link to the add-on's display page. PicLens has different .xpi's for different platforms, so we also link to their display page.

Comment 2

[Justin Scott [:fligtar]]

Attachment: patch for mozilla.com fx 3 branch

Comment 3

[Justin Scott [:fligtar]]

Attachment: patch for AMO

Comment 4

[Reed Loden [:reed]]

Comment on attachment 323800 [details] [diff] [review]
patch for mozilla.com fx 3 branch

>+    // AMO URLs used on this page
>+    $install_url = "https://addons.mozilla.org/services/install.php?addon_id=";
>+    $display_url = "https://addons.mozilla.org/{$lang}/firefox/addon/";

Should be using https://<?=$lang?>.add-ons.mozilla.com/... for both of these.

> 				<li><a href="http://www.mozilla.com/en-US/add-ons/ebay/features.html">View the Tutorial</a></li>

I know it's not your fault, but can you change that to use "/<?=$lang?>/add-ons/ebay/features.html", please?

>+			<a href="https://addons.mozilla.org/<?=$lang?>/firefox/"><img src="/img/tignish/add-ons/getpersonal/icon-more.png" alt="Get More Add-ons" height="100" width="150" /></a>

https://<?=$lang?>.add-ons.mozilla.com/<?=$lang?>/firefox/, please.

Comment 5

[Justin Scott [:fligtar]]

Comment on attachment 323800 [details] [diff] [review]
patch for mozilla.com fx 3 branch

I discussed this with Wil, and we decided to use addons.mozilla.org links.

Comment 6

[Wil Clouser [:clouserw]]

Comment on attachment 323801 [details] [diff] [review]
patch for AMO

It seems like this regex is vulnerable to something like a referrer like https://mozilla.com.evilhost.com or https://localhost.evil.com/

r+ because this isn't a new thing though

Comment 7

[Wil Clouser [:clouserw]]

like

Comment 8

[Justin Scott [:fligtar]]

Thanks for the review.

I committed the AMO patch, but don't have commit access to the mozilla.com firefox 3 branch, so can you check it in for me Wil?

First, you'll need to revert reed's change to customize/getpersonal/index.html since he decided to change all the same lines while we were discussing the URL issue, causing 14 conflicts with my patch that was actually reviewed.

svn merge -r HEAD:14726 index.html

Comment 9

[Reed Loden [:reed]]

Be nice, fligtar.

Sending        en-US/firefox/customize/getpersonal/index.html
Transmitting file data .
Committed revision 14736.

Comment 10

[Justin Scott [:fligtar]]

The patch that you checked in is not the patch that I submitted, it's not the patch that was reviewed, and it's not using the URL format that I, clouserw, and morgamic all said we would prefer in IRC.

It really encourages me to work on mozilla.com bugs like this.

Comment 11

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

Verified FIXED, since I've already verified that this functionality works over in bug 433408.

Comment 12

[Michael Morgan [:morgamic]]

(In reply to comment #10)
> The patch that you checked in is not the patch that I submitted, it's not the
> patch that was reviewed, and it's not using the URL format that I, clouserw,
> and morgamic all said we would prefer in IRC.
> 
> It really encourages me to work on mozilla.com bugs like this.
Reed - don't ever do this again.  If you do, you won't have commit access to mozilla.com anymore.