290958 - Must hack url to get Thunderbird XPI

Status: RESOLVED DUPLICATE of bug 291616

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[Arthur Wiebe]

When reviewing extensions you must hack the url in order to get the XPI for
Thunderbird extensions.

Comment 1

[alanjstr]

http://lxr.mozilla.org/update1.0/source/developers/approval.php#155

These lines will need to be reordered a bit to retain whether it was for
Thunderbird and change the install link to a pure download link for both themes
and extensions.

Comment 2

[Arthur Wiebe]

Yeah, will attach a patch soon

Comment 3

[Arthur Wiebe]

Attachment: Fixes Tb install links

this patch should worth although I'm not able to test it. I would need an SQL
dump for testing something like this.

Comment 4

[alanjstr]

Comment on attachment 181137 [details] [diff] [review]
Fixes Tb install links

>      while ($row3 = mysql_fetch_array($sql_result3)) {
>   echo"".ucwords($row3[shortname])." $row3[MinAppVer]-$row3[MaxAppVer] \n";
>+     $shortName = $row3['shortname'];
>     }

You're in the middle of a while loop.  What guarantee is there that you'll get
the one you want as last?

>+    switch ($shortName) {
>+        case 'tb':
>+        case 'fx':
>     }

We have more applications than just TB and FX.	Mozilla 1.x also supports
triggers, although not necesarily the same format (I can't remember).  Nvu and
Sunbird will need download links when they come along.

Comment 5

[Arthur Wiebe]

heh, one of my crazy mistakes. (What happens when your in a hurry)

Comment 6

[Arthur Wiebe]

Why is there a loop like this in the first place?
>> while ($row3 = mysql_fetch_array($sql_result3)) {

It should always return something and go through a single cycle. Why not change
it to:

$row3 = mysql_fetch_array($sql_result3);
echo"".ucwords($row3[shortname])." $row3[MinAppVer]-$row3[MaxAppVer] \n";

because it's only expecting one row in the first place. And it will return a row
unless there is somthing really wrong.

I'll make the Suite and Fx the same. Nvu should probably be the same way as Tb
but I don't know what the Nvu shortname will be so it will just use the default
link until that can be changed.

Comment 7

[Arthur Wiebe]

Attachment: Patch 2

This patch fixes problems.

I made Tb have links straight to XPI but everything else is the same.
Sunbird and Nvu will probably be added after the 2.0 rewrite anyway.

I also fixed a mismatching span as a bonus :)

Comment 8

[alanjstr]

Comment on attachment 181159 [details] [diff] [review]
Patch 2


>-     while ($row3 = mysql_fetch_array($sql_result3)) {
>-  echo"".ucwords($row3[shortname])." $row3[MinAppVer]-$row3[MaxAppVer] \n";
>-    }
>+    $row3 = mysql_fetch_array($sql_result3);
>+    echo ucwords($row3['shortname'])." $row3[MinAppVer]-$row3[MaxAppVer] \n";
>+    

You can't get rid of the while loop.  If something works for multiple things,
like FX and TB, you need to list them.

try 

while () {
...
if (ucwords($row3['shortname']) == 'TB') { hasTB = true }
}

Comment 9

[Arthur Wiebe]

Attachment: Loop readded

OK, I knew there was a catch which is why I didn't set the review flag on that
one.

Well I think this one is ready to go. Assuming the shortname for Nvu will be nv
I added support for it as well.

Sunbird can come later.

Comment 10

[alanjstr]

Comment on attachment 181180 [details] [diff] [review]
Loop readded

OK, looks good to me, with one caveat:	you should lowercase the value you're
comparing to make sure that you never run into Tb or TB

Comment 11

[Arthur Wiebe]

uh, caveat not understood. It is lowercased.

Comment 12

[Arthur Wiebe]

ok, now I understand. Your caveat. But I don't really think it's possible to get
a Tb, or TB.

Comment 13

[Arthur Wiebe]

Attachment: Forces lowercase string check

Comment 14

[Arthur Wiebe]

Comment on attachment 181187 [details] [diff] [review]
Forces lowercase string check

This patch gives direct link to XPI even if it's browser compatible and only
checks for Thunderbird. Will work on it some more.

Comment 15

[Arthur Wiebe]

Attachment: Make sure it's not a browser

This patch makes sure it's not a browser before changing the link. Also it
changes the link to Download Now for non browser extensions.

Comment 16

[Michael Morgan [:morgamic]]

Comment on attachment 181591 [details] [diff] [review]
Make sure it's not a browser

Improper use of in_array() causes warnings:
> +    if (!in_array('fx') && !in_array('mz')) {

Improper use of \n in download link:
> +        echo '<span><a href="' . $installink . '">( Install Now )</a></span>\n"';

Comment 17

[Arthur Wiebe]

Attachment: Fixes in_array() args

in_array() arguments fixed.

I will no longer be submitting patches because it's a hopeless can if you can't
test anything.

Comment 18

[Arthur Wiebe]

Let me just expound on that a little. I decided to not submit patches anymore
because it's not very productive if I cannot test my own patches.
The in_array() problem was just a result of that.

Comment 19

[alanjstr]

Artooro -

Have you requested a dev environment on chameleon?

Comment 20

[Arthur Wiebe]

alanjstr, yes I have. But have not recieved reply from kveton in about a week or so.

Comment 21

[Michael Morgan [:morgamic]]

*** This bug has been marked as a duplicate of 291616 ***