485802 - Bunch of minor tweaks/fixes in svn.mozilla.org/addons/trunk

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: API, enhancement, P3)

Description

[Aleksey Lim]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090329 Gentoo Shiretoko/3.1b3
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090329 Gentoo Shiretoko/3.1b3

Within activities.sugarlabs.org I've made some minor tweaks/fixes in addons trunk code, I think it could be useful for upstream
(attached patches use a couple of new constants like EDITOR_EMAIL)

Reproducible: Always

Comment 1

[Aleksey Lim]

Attachment: Do not let user delete last application tag

Comment 2

[Aleksey Lim]

Attachment: Do not let user save empty GUID

Comment 3

[Aleksey Lim]

Attachment: Do not overlap footer by right panel

Comment 4

[Aleksey Lim]

Attachment: Do not publish activities with empty application list; remove hardcoded strings

Comment 5

[Aleksey Lim]

Attachment: Do not recommend empty activities in "We Recommend" list

Comment 6

[Aleksey Lim]

Attachment: Fix misplaced "any" option for versions in search panel

Comment 7

[Aleksey Lim]

Attachment: Fix serach by platform

Comment 8

[Aleksey Lim]

Attachment: Fix share links

Comment 9

[Aleksey Lim]

Attachment: Fix "Undefined offset:" notice on "Add-on Categories" page

Comment 10

[Aleksey Lim]

Attachment: Fix wrong href to files

Comment 11

[Aleksey Lim]

Attachment: Make sure current tag isnt empty while browse addons

Comment 12

[Aleksey Lim]

Attachment: Remove hardcoded strings

Comment 13

[Aleksey Lim]

Attachment: Remove link to old developers interface from nav header; remove hardcoded strings.patch

Comment 14

[Aleksey Lim]

Attachment: Suppress errors if no totals for category

Comment 15

[Aleksey Lim]

Attachment: Suppress key error in Preview Manager page; do not let user save empty GUID, do not publish activities with empty application list

Comment 16

[Aleksey Lim]

Attachment: Take into account "all" category in browse

Comment 17

[Aleksey Lim]

Attachment: Use category in random list of addons

Comment 18

[Wil Clouser [:clouserw]]

Hey Aleksey, these are awesome but we're having a hard time integrating some of the bigger ones without tests.  Did you write any tests for them that you could attach also?

I'm reassigning this to nobody@ to let people pick up patches one at a time.

Comment 19

[Wil Clouser [:clouserw]]

Comment on attachment 369904 [details] [diff] [review]
Suppress errors if no totals for category

r24458, Thanks.

Comment 20

[Wil Clouser [:clouserw]]

Comment on attachment 369898 [details] [diff] [review]
Fix share links

r-, not because the patch isn't good but that we already changed this in production slightly differently but with the same effect.

Comment 21

[Aleksey Lim]

(In reply to comment #18)
> Hey Aleksey, these are awesome but we're having a hard time integrating some of
> the bigger ones without tests.  Did you write any tests for them that you could
> attach also?
> 
> I'm reassigning this to nobody@ to let people pick up patches one at a time.

well, in my mind it wasn't a big change, so I tested patches by hands :)
but if you point me to the most vulnerable places I'll try to write tests

Comment 22

[Jeff Balogh (:jbalogh)]

(As Wil said, thanks a bunch for all these patches!)

(In reply to comment #1)
> Created an attachment (id=369890) [details]
> Do not let user delete last application tag

Showing the errors is probably a great idea, but how does it prevent deleting the last tag?

Comment 23

[Aleksey Lim]

(In reply to comment #22)
> (As Wil said, thanks a bunch for all these patches!)
> 
> (In reply to comment #1)
> > Created an attachment (id=369890) [details] [details]
> > Do not let user delete last application tag
> 
> Showing the errors is probably a great idea, but how does it prevent deleting
> the last tag?

https://bug485802.bugzilla.mozilla.org/attachment.cgi?id=369905
when I posted patches I followed per-file rule

Comment 24

[Jeff Balogh (:jbalogh)]

(In reply to comment #3)
> Created an attachment (id=369892) [details]
> Do not overlap footer by right panel

I'm not sure where the footer is overlapping the right panel.  Actually, I don't even know what the right panel is. :)

I think you added margin-bottom in there, but it has a value of '25 ex'.  The rule right before that is 'margin: 0 auto 1em'.  The 1em gets applied to margin-bottom (http://www.dustindiaz.com/css-shorthand/); is that not sufficient?

Comment 25

[Jeff Balogh (:jbalogh)]

(In reply to comment #23)
> (In reply to comment #22)
> > (As Wil said, thanks a bunch for all these patches!)
> > 
> > (In reply to comment #1)
> > > Created an attachment (id=369890) [details] [details] [details]
> > > Do not let user delete last application tag
> > 
> > Showing the errors is probably a great idea, but how does it prevent deleting
> > the last tag?
> 
> https://bug485802.bugzilla.mozilla.org/attachment.cgi?id=369905
> when I posted patches I followed per-file rule

Ah.  Are there any other patches that belong together?  If they're meant to work together, patching it all at the same time is a big help.

If you just want to list which go together, I'll break them out into separate bugs (since things are already getting jumbled here :).

Comment 26

[Aleksey Lim]

(In reply to comment #24)
> (In reply to comment #3)
> > Created an attachment (id=369892) [details] [details]
> > Do not overlap footer by right panel
> 
> I'm not sure where the footer is overlapping the right panel.  Actually, I
> don't even know what the right panel is. :)
> 
> I think you added margin-bottom in there, but it has a value of '25 ex'.  The
> rule right before that is 'margin: 0 auto 1em'.  The 1em gets applied to
> margin-bottom (http://www.dustindiaz.com/css-shorthand/); is that not
> sufficient?

well, I couldn't find better solution(I don't know css/html stuff well)
but this(dirty?) hack works for me

you can find this issue on https://addons.mozilla.org/en-US/firefox/ -- in my browser(Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090412 Gentoo Shiretoko/3.1b3) I can't see the whole "Recently Updated:" section at the right-bottom corner

Comment 27

[Aleksey Lim]

(In reply to comment #25)
> Ah.  Are there any other patches that belong together?
yup, my fault 

>  If they're meant to
> work together, patching it all at the same time is a big help.
>
>  If you just want to list which go together, I'll break them out into separate
> bugs (since things are already getting jumbled here :).
let me revise patches, there shouldn't be much cross patches

Comment 28

[Jeff Balogh (:jbalogh)]

(In reply to comment #7)
> Created an attachment (id=369897) [details]
> Fix serach by platform

I'm not sure what this is doing/why it works.  Could you add a comment or test please?

Comment 29

[Aleksey Lim]

(In reply to comment #28)
> (In reply to comment #7)
> > Created an attachment (id=369897) [details] [details]
> > Fix serach by platform
> 
> I'm not sure what this is doing/why it works.  Could you add a comment or test
> please?

on http://activities.sugarlabs.org/ we have different platforms(platform names)
because of that I found this bug

the problem is: list of platforms in old code is sorted by name, with ours platform names we got discrepancy between order number in list and values of PLATFORM_* for particular platform, so i just removed sorting by name

Comment 30

[Aleksey Lim]

(In reply to comment #29)
> (In reply to comment #28)
> > (In reply to comment #7)
> > > Created an attachment (id=369897) [details] [details] [details]
> > > Fix serach by platform
> > 
> > I'm not sure what this is doing/why it works.  Could you add a comment or test
> > please?
> 
> on http://activities.sugarlabs.org/ we have different platforms(platform names)
> because of that I found this bug
> 
> the problem is: list of platforms in old code is sorted by name, with ours
> platform names we got discrepancy between order number in list and values of
> PLATFORM_* for particular platform, so i just removed sorting by name

and also.. in this patch $pid is equal to PLATFORM_* value(except 0 value - it means any platform).. to be honest I don't remember the exact old behaviour but remember it was wrong in our case

Comment 31

[Aleksey Lim]

(In reply to comment #26)
> If you just want to list which go together, I'll break them out into separate
> bugs (since things are already getting jumbled here :).
I've obsoleted all cross patches and created #488612 and #488614

Comment 32

[u278084]

Comment on attachment 369896 [details] [diff] [review]
Fix misplaced "any" option for versions in search panel

A victim of bit-rot. We now render the elements/amo2009/search.thtml file.

Comment 33

[u278084]

Comment on attachment 369892 [details] [diff] [review]
Do not overlap footer by right panel

I think this went away with the new theme.

Comment 34

[u278084]

Comment on attachment 369900 [details] [diff] [review]
Fix wrong href to files

Can you explain this? By default, FILES_URL already contains 'file/' in the config.php

This works for me without the patch applied.

Comment 35

[u278084]

Comment on attachment 369902 [details] [diff] [review]
Remove hardcoded strings

Please add EDITOR_EMAIL and SITE_IRC, and other php defines in config.php

I'll r+ it with those changes.

Comment 36

[Fred Wenzel [:wenzel]]

We won't need to incorporate these patches into Remora anymore, but we'd like to thank you, Aleksey, for the hard work and good patches you provided. If in the future you guys are considering moving to Zamboni, you are very welcome to be involved in that project as well!