Make fuller use of GetStringPref in suite

RESOLVED FIXED in seamonkey2.4

Status

SeaMonkey
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ian Neal, Assigned: Ian Neal)

Tracking

Trunk
seamonkey2.4
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 540485 [details] [diff] [review]
switch more of suite to use GetStringPref

There are various parts of the suite code that could use GetStringPref helper from utilityOverlay.js
This patch:
* Tweaks GetStringPref helper to take a second argument for optional use.
* Switches more of the suite code to use GetStringPref helper.
Attachment #540485 - Flags: review?(neil)
(Assignee)

Comment 1

7 years ago
Created attachment 540607 [details] [diff] [review]
switch more of suite to use GetStringPref v2

Changes since last version:
* Fixed an issue where a try/catch had been hiding a missing Services.
Attachment #540485 - Attachment is obsolete: true
Attachment #540607 - Flags: review?(neil)
Attachment #540485 - Flags: review?(neil)

Comment 2

7 years ago
Comment on attachment 540607 [details] [diff] [review]
switch more of suite to use GetStringPref v2

>+  url = GetStringPref("sidebar.customize.all_panels.url", "about:blank");
If this is the only user, this could be written
url = GetStringPref("sidebar.customize.all_panels.url") || "about:blank";

>         if (LDAPSession) {
>-            let url = sPrefs.getComplexValue(autocompleteDirectory +".uri",
>-              Components.interfaces.nsISupportsString).data;
>+            let url = GetStringPref(autocompleteDirectory +".uri");
I don't really want to fiddle with this code: it's slated to be ripped out and replaced with a toolkit-based autocomplete component, see bug 452232. r- because of that.
Attachment #540607 - Flags: review?(neil) → review-
(Assignee)

Comment 3

7 years ago
Created attachment 541921 [details] [diff] [review]
switch more of suite to use GetStringPref without LDAP [Checked in: Comment 6]

Changes since previous version:
* No tweaking of GetStringPref now.
* Removed LDAP changes.
* Made suggested changes to sidebar and inlined variable declaration.
Attachment #540607 - Attachment is obsolete: true
Attachment #541921 - Flags: review?(neil)

Comment 4

7 years ago
Comment on attachment 541921 [details] [diff] [review]
switch more of suite to use GetStringPref without LDAP [Checked in: Comment 6]

>-    url = urlFormatter.formatURL(url);
[Oops, looks like urlFormatter was never declared?]
Attachment #541921 - Flags: review?(neil) → review+
(Assignee)

Comment 5

7 years ago
Comment on attachment 541921 [details] [diff] [review]
switch more of suite to use GetStringPref without LDAP [Checked in: Comment 6]

http://hg.mozilla.org/comm-central/rev/322b0049a5a7
Attachment #541921 - Attachment description: switch more of suite to use GetStringPref without LDAP → switch more of suite to use GetStringPref without LDAP [Checked in: Comment 5]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.4
(Assignee)

Comment 6

7 years ago
Comment on attachment 541921 [details] [diff] [review]
switch more of suite to use GetStringPref without LDAP [Checked in: Comment 6]

Backed out incorrect patch:
http://hg.mozilla.org/comm-central/rev/18cdf1e1fc2b
Correct patch checked in:
http://hg.mozilla.org/comm-central/rev/e7d8e36b4769
Attachment #541921 - Attachment description: switch more of suite to use GetStringPref without LDAP [Checked in: Comment 5] → switch more of suite to use GetStringPref without LDAP [Checked in: Comment 6]
You need to log in before you can comment on or make changes to this bug.