325440 - Sunbird's GUID isn't recognized ("Get more themes" link in Sunbird's themes window shows Firefox themes page)

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[Simon Paquet [:sipaq]]

When clicking on the "Get more themes" link in Sunbird's themes window, I get delegated to 
https://addons.mozilla.org/themes/?application={ec8030f7-c20a-464f-9b0e-13a3a9e97384} aka https://addons.mozilla.org/themes/?application=firefox

Instead Sunbird's URL https://addons.mozilla.org/themes/?application={718e30fb-e89b-41dd-9da7-e25a45638b28} should load the https://addons.mozilla.org/themes/?application=sunbird webpage.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Hm, strange. The dialog gets the app ID from nsIXULRuntime, which for Sunbird seems to be implemented correctly.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/extensions/content/extensions.js&rev=1.85#194
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/calendar/sunbird/app/nsCalendarApp.cpp&rev=1.18&mark=55#48

Comment 2

[Simon Paquet [:sipaq]]

Gavin, why did you move this bug to Calendar from Update? What is wrong on our side?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Sorry, you said "I get sent to amo?application={ec8030f7-c20a-464f-9b0e-13a3a9e97384}" when you meant "I get sent to amo?application={718e30fb-e89b-41dd-9da7-e25a45638b28} which gets treated like amo?application=firefox", which led me to believe it was a client side bug.

Comment 4

[alanjstr]

UMO 1.0
http://lxr.mozilla.org/update1.0/source/core/inc_guids.php#39

AMO 2.0 
Can't find it.  morgamic, do we have this redirect?  This should be database driven.

Comment 5

[Michael Morgan [:morgamic]]

Why are we passing GUIDs as GET parameters?

Comment 6

[alanjstr]

The applications pass them to us.  They know their own GUID.  We convert that into a name.

Comment 7

[Michael Morgan [:morgamic]]

Why are we passing GUIDs as GET parameters?

Comment 8

[Michael Morgan [:morgamic]]

Oops.  Thanks for the tip Alan.

I didn't realize we passed the GUID from the Get More Themes... link.  My question is really about why we are passing the GUID instead of an application name?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #8)
> I didn't realize we passed the GUID from the Get More Themes... link.  My
> question is really about why we are passing the GUID instead of an application
> name?

Presumably because the GUID for a given application is much more consistent than it's name. Some Firefox builds are not "Firefox", for example. Knowing which keyword AMO responds to for a given app is more complicated than just giving it a GUID.

Comment 10

[Simon Paquet [:sipaq]]

(In reply to comment #4)
> UMO 1.0
> http://lxr.mozilla.org/update1.0/source/core/inc_guids.php#39

Would a patch for this still be accepted or should I not bother?

> AMO 2.0 
> Can't find it.  morgamic, do we have this redirect?  This should be database
> driven.

There are some instances in the code, which make assumptions about the possible applications and none of these instances knows Sunbird:

http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/htdocs/extensions.php#27
http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/inc/init.php#53
http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/htdocs/index.php#28
http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/htdocs/recommended.php#23
http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/htdocs/rss.php#20
http://landfill.mozilla.org/mxr-test/webtools/source/addons/public/htdocs/themes.php#25

Comment 11

[Michael Morgan [:morgamic]]

Those instances should be replaced with a wrapper func in amo.class.php.

Writing a v1.0 patch would not be a good use of time, but a v2.0 patch would be welcome, if you want to.

I'll add this to the v2.0 todo list.

Comment 12

[Simon Paquet [:sipaq]]

Morgamic, this seems to be fixed with the new AMO 2.0 system.
Although the header still shows the "Firefox Addons" header. I guess it's time for Sunbird to have its own section.

Should I open a new bug for that or can we track that here?

Comment 13

[Cameron]

(In reply to comment #12)
> Morgamic, this seems to be fixed with the new AMO 2.0 system.
> Although the header still shows the "Firefox Addons" header. I guess it's time
> for Sunbird to have its own section.
> 
> Should I open a new bug for that or can we track that here?

See Bug 332417

Comment 14

[Cameron]

(In reply to comment #12)
> This seems to be fixed with the new AMO 2.0 system.

Seems to be, or is?

(-> FIXED?)

Comment 15

[Michael Morgan [:morgamic]]

Yes, this was fixed by changes Wil made to the app switching.  The header logo doesn't change, but I wouldn't worry about that too much in the short term since there is bug 337198 which is slated to modify app switching behavior to avoid this kind of confusion.

Comment 16

[Omar Bajraszewski]

It's not working again- 'get themes/extensions' link shows firefox extensions/themes pages. Please reopen the bug

Comment 17

[Omar Bajraszewski]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060911 Calendar/0.3a2+

Comment 18

[Cameron]

(In reply to comment #16)
> It's not working again- 'get themes/extensions' link shows firefox
> extensions/themes pages. Please reopen the bug
> 

If you read this bug carefully, you'll notice that despite the header image showing "Firefox add-ons," the add-ons listed are actually Sunbird Add-ons.

Am I incorrect?

Comment 19

[Omar Bajraszewski]

Not really. 
https://addons.mozilla.org/sunbird/extensions/
shows extensions that are not compatible with sunbird. 

Comment 20

[Omar Bajraszewski]

https://bugzilla.mozilla.org/show_bug.cgi?id=352641#c1