Last Comment Bug 530099 - Remove extensions.getMore* preferences
: Remove extensions.getMore* preferences
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 21.0
Assigned To: Javi Rueda
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-20 09:28 PST by Dave Townsend [:mossop]
Modified: 2013-01-26 09:48 PST (History)
11 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch for m-c (4.29 KB, patch)
2012-09-06 06:17 PDT, Javi Rueda
dtownsend: review+
Details | Diff | Splinter Review
Patch for c-c (1.54 KB, patch)
2012-12-03 10:21 PST, Javi Rueda
bwinton: review+
Details | Diff | Splinter Review

Description Dave Townsend [:mossop] 2009-11-20 09:28:01 PST
When you move to the 1.9.2 branch you should look at removing some/all of the extensions.getMore* preferences as these will once again make links appear in the add-ons manager. For Firefox we have removed all but the getMoreThemes link as we want a link to download themes in addition to the Get Add-ons panel in the manager.
Comment 1 Robert Kaiser 2009-11-30 12:32:38 PST
SeaMonkey uses this pref elsewhere (View > Apply Theme > Get New Themes), we can't just remove this.
Comment 2 Javi Rueda 2012-09-04 02:55:35 PDT
The only pref kept on last Daily (18) is extensions.getMoreThemesURL.
Comment 3 Mark Banner (:standard8) 2012-09-04 11:54:32 PDT
(In reply to Javi Rueda from comment #2)
> The only pref kept on last Daily (18) is extensions.getMoreThemesURL.

Actually, afaict this isn't used now, and doesn't need to be in FF any more neither.
Comment 4 Javi Rueda 2012-09-04 13:14:39 PDT
http://mxr.mozilla.org/comm-central/search?string=extensions.getMoreThemesURL

It seems ok to remove it from the comm-central repository (/calendar and /mail only) and from the mozilla-central.
Comment 5 Javi Rueda 2012-09-06 06:17:57 PDT
Created attachment 658861 [details] [diff] [review]
patch for m-c

Removes references for getMore* preference on the mozilla-central repository.

This patch also modifies a JS-Shell regression test for bug 308111. I tried to run an automated testing and it didn't show any message about failing on this specific test.

Those preferences will be removed also on the comm-central repository (sunbird and thunderbird).

Changes for seamonkey are under research. Opinions on this?

Thank you, Ehsan.
Comment 6 :Ehsan Akhgari 2012-09-06 07:24:08 PDT
Comment on attachment 658861 [details] [diff] [review]
patch for m-c

I know absolutely nothing about what this pref does!
Comment 7 Javi Rueda 2012-09-17 11:36:57 PDT
My computer is down -probably power supply broken- so, to I am requesting for check-in the patch in order to avoid problems in case the code changes and could not be applied.

It is still needed to make the change in comm-central. i am unsure if a patch could also be created for suite as the string seems to be used on code.

If anybody wants to work on this, feel free to take it -without asking- if there were no news from me :-)
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-09-17 15:43:20 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/414b774c927b
Comment 9 Graeme McCutcheon [:graememcc] 2012-09-18 05:05:57 PDT
https://hg.mozilla.org/mozilla-central/rev/414b774c927b
Comment 10 Javi Rueda 2012-11-21 12:23:48 PST
The string also appears in the IM directory in http://mxr.mozilla.org/comm-central/source/mail/components/im/content/imAccountWizard.js#8

I thought it could be a placeholder, but, as I was unsure, I asked on the IRC. This was what clokep answered me.

21:15:40 - clokep: kekkyojin: I'm pretty sure that that preference is supposed to point to https://addons.mozilla.org/en-US/thunderbird/addon/additional-chat-protocols/, can you CC :clokep and :florian to that bug and ask in the bug? (florian should know)

Then, :florian and :clokep, what do you think about that? Should it be removed or be kept?
Comment 11 Javi Rueda 2012-12-03 10:21:07 PST
Created attachment 687846 [details] [diff] [review]
Patch for c-c

Hi, Blake. It seems all patches I submit could be reviewed by you :-\ :-/

Could you look at this one?

I tested it by opening the Add-ons Manager from the main menu and all seems to be working right and there were no eror messages at the Error Console.

Thank you.
Comment 12 Florian Quèze [:florian] [:flo] (PTO until August 29th) 2012-12-03 10:40:47 PST
(In reply to Javi Rueda from comment #10)

> Then, :florian and :clokep, what do you think about that? Should it be
> removed or be kept?

It's mostly a UX decision, so Blake can decide. Either we need to add the pref to make the "Show more protocols" link appear in the account wizard, or we can remove the dead code if we don't want to have that link in the UI. You can also just not change anything.
If you are going to remove this code, or are adding a value to the pref, please request review from me on the patch doing that.
Comment 13 Javi Rueda 2012-12-18 14:04:36 PST
Patch in attachment 687846 [details] [diff] [review] doesn't include any code for the Chat component in Thunderbird. That part will be seen later.
Comment 14 Blake Winton (:bwinton) (:☕️) 2013-01-01 11:18:25 PST
Comment on attachment 687846 [details] [diff] [review]
Patch for c-c

(In reply to Florian Quèze [:florian] [:flo] from comment #12)
> (In reply to Javi Rueda from comment #10)
> It's mostly a UX decision, so Blake can decide. Either we need to add the
> pref to make the "Show more protocols" link appear in the account wizard, or
> we can remove the dead code if we don't want to have that link in the UI.
> You can also just not change anything.

I would be happy to see a "Show more protocols" link in the chat acount wizard, but I think that we should leave this change for a future patch.

I'm happy with the patch itself, but it would be nice to wait for a clean try-server run before we check it in.
(I've kicked one off at https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=a1cc305a2675 for you.)

Thanks, and I apologize for the delay in reviewing this patch.
Blake.
Comment 15 Javi Rueda 2013-01-04 05:04:11 PST
There were some orange statuses on the try-server. Looking at the reports with such a colour, there were many "intermitent oranges". As I am unsure if that is the way it should be, I prefer to wait before asking for a check-in.

A new try-server run would be needed, Blake?

On the other hand, no problem about the delay. These days are always very busy ones :-)
Comment 16 Blake Winton (:bwinton) (:☕️) 2013-01-04 09:25:06 PST
I think we're probably safe, since they are intermittent, and since they seem unrelated to the change, but let's double-check with Standard8 before landing…

Thanks,
Blake.
Comment 17 Mark Banner (:standard8) 2013-01-24 02:42:30 PST
Yeah, just intermittent, we're good to go here. Sorry for the delay in looking at this.
Comment 18 Ryan VanderMeulen [:RyanVM] 2013-01-26 09:48:38 PST
https://hg.mozilla.org/comm-central/rev/26e91802d679

Note You need to log in before you can comment on or make changes to this bug.