Closed Bug 1547041 Opened 1 year ago Closed 1 year ago

Update and tidy SeaMonkey application preferences

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(seamonkey2.49esr unaffected, seamonkey2.53+ fixed, seamonkey2.57esr+ fixed, seamonkey2.63 wontfix)

RESOLVED FIXED
seamonkey2.66
Tracking Status
seamonkey2.49esr --- unaffected
seamonkey2.53 + fixed
seamonkey2.57esr + fixed
seamonkey2.63 --- wontfix

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

Details

Attachments

(10 files, 8 obsolete files)

18.47 KB, patch
Details | Diff | Splinter Review
11.37 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
25.90 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
11.42 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
25.90 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
16.73 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
16.75 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
25.93 KB, patch
frg
: review+
Details | Diff | Splinter Review
16.72 KB, patch
frg
: review+
Details | Diff | Splinter Review
11.47 KB, patch
frg
: review+
Details | Diff | Splinter Review

As pointed out by frg, the preferences part of bug 1297686 was not ported as part of bug 1544222. This bug ports parts of the following bugs:
Part 1:

  • Bug 1408777 - Automatically fix instances of missing semicolons in the tree
  • Bug 1486739 - Add missing dangling commas in browser/, services/, taskcluster/ and toolkit/
  • Replace foobar: function() with foobar()

Part 2:

  • Bug 400472 - Applications pref pane displays duplicate entries after selection
  • Bug 402252 - application details should be accessible from prefs UI
  • Bug 416899 - invalid handlers listed in the Application Details dialog
  • Bug 480242 - "Always ask" doesn't work for Plugins
  • Bug 527433 - nsIWebContentConverterRegistrar::getContentHandlers count out param should be optional
  • Bug 1111540 - setting app in about:preferences should also work on Windows
  • Bug 1294989 - Enable eslint for browser/components/preferences
  • Bug 1412445 - replace custom QI impl with XPCOMUtils.generateQI call
  • Bug 1339461 - script-generated patch to convert foo.indexOf(...) == -1 to foo.includes()
  • Bug 1436575 - Autofix errors from no-compare-against-boolean-literal
  • Bug 1456035 - Part 5 - Convert manual QueryInterface to ChromeUtils.generateQI
  • Bug 1457027 - Part 3 - Unify references to bundlePreferences

Part 3:

  • Bug 1457027 - Part 2 - Define services using defineLazyServiceGetters

Part 4:

  • Bug 1297686 - List GIO handlers in the protocol handler list, in the handlers dialog, and in preferences

For comm-central
Part 5:

  • Bug 1453345 - part 5. Remove pointless JS implemenations of QI to nsIDOMEventListener
  • Bug 1456035 - Part 4 - Convert callers of XPCOMUtils.generateQI to ChromeUtils.generateQI
  • Bug 1484496 - Part 5a - Convert browser/ nsISimpleEnumerator users to use JS iteration
  • Bug 1485426 - Use createXULElement instead of createElement in XUL docs
  • Bug 1486182 - Part 2a - Add Services.catMan getter for the category manager
  • Bug 1486249 - Part 2 - Convert JS nsISimpleEnumerator implements to JS enumerators
  • Bug 1488908 - QI the elements of nsIGIOservice.getAppsForURIScheme() to nsIHandlerApp
  • Bug 1457027 - Part 4 - Use class syntax for the HandlerInfoWrapper hierarchy
  • Bug 1457027 - Part 5 - Move _describeType to HandlerInfoWrapper
  • Bug 1457027 - Part 6 - Move _describePreferredAction to HandlerInfoWrapper
  • Bug 1457027 - Part 7 - Move action icon getters to HandlerInfoWrapper
Attached patch Part 1 for esr60 (obsolete) — Splinter Review
Attachment #9060783 - Attachment description: Patch 1 for esr60 → Part 1 for esr60
Attached patch Part 2 for esr60 (obsolete) — Splinter Review
Attached patch Part 3 for esr60 (obsolete) — Splinter Review
Attached patch Part 4 for esr60 (obsolete) — Splinter Review
Attached patch Part 1 for cc (obsolete) — Splinter Review
Attached patch Part 2 for cc (obsolete) — Splinter Review
Attached patch Part 3 for cc (obsolete) — Splinter Review
Attached patch Part 4 for cc (obsolete) — Splinter Review
Attached patch Part 5 for ccSplinter Review
Attachment #9060783 - Flags: review?(frgrahl)
Attachment #9060783 - Flags: approval-comm-esr60?
Attachment #9060784 - Flags: review?(frgrahl)
Attachment #9060784 - Flags: approval-comm-esr60?
Attachment #9060785 - Flags: review?(frgrahl)
Attachment #9060785 - Flags: approval-comm-esr60?
Attachment #9060787 - Flags: review?(frgrahl)
Attachment #9060788 - Flags: review?(frgrahl)
Attachment #9060790 - Flags: review?(frgrahl)
Comment on attachment 9060786 [details] [diff] [review]
Part 4 for esr60

Part 4 has been done in Bug 1559255
Attachment #9060786 - Attachment is obsolete: true
Comment on attachment 9060791 [details] [diff] [review]
Part 4 for cc

Part 4 has been done in Bug 1559255
Attachment #9060791 - Attachment is obsolete: true
Comment on attachment 9060783 [details] [diff] [review]
Part 1 for esr60

> suite/components/pref/content/pref-applications.js
>  __proto__: HandlerInfoWrapper.prototype,

According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto

__proto__ is deprecated. Do we need to add it at this stage?

r/a+ with this answered.
Attachment #9060783 - Flags: review?(frgrahl)
Attachment #9060783 - Flags: review+
Attachment #9060783 - Flags: approval-comm-esr60?
Attachment #9060783 - Flags: approval-comm-esr60+
Comment on attachment 9060784 [details] [diff] [review]
Part 2 for esr60

> suite/components/pref/content/pref-applications.js

> QueryInterface: XPCOMUtils.generateQI(["nsIMutableArray", "nsIArray"]),

 Shouldn't this be Ci.nsIMutableArray and Ci.nsIArray?

> +  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
> +                                         Ci.nsIDOMEventListener]),

General question. Why is checking nsISupports no longer neccessary. Happy to do it over irc.

r/a+ with first item fixed or answered.
Attachment #9060784 - Flags: review?(frgrahl)
Attachment #9060784 - Flags: review+
Attachment #9060784 - Flags: approval-comm-esr60?
Attachment #9060784 - Flags: approval-comm-esr60+
Comment on attachment 9060785 [details] [diff] [review]
Part 3 for esr60

> suite/components/pref/content/pref-applications.js

> +  gWebContentContentConverterService: ["@mozilla.org/embeddor.implemented/web-content-handler-registrar;1", "nsIWebContentConverterService"],

Typo and should be gWebContentConverterService I assume?

r/a+ with variable fixed.
Attachment #9060785 - Flags: review?(frgrahl)
Attachment #9060785 - Flags: review+
Attachment #9060785 - Flags: approval-comm-esr60?
Attachment #9060785 - Flags: approval-comm-esr60+
Comment on attachment 9060787 [details] [diff] [review]
Part 1 for cc

r+ with the items from the esr60 version addressed here too..
Attachment #9060787 - Flags: review?(frgrahl) → review+
Comment on attachment 9060788 [details] [diff] [review]
Part 2 for cc

r+ with the items from the esr60 version addressed here too.
Attachment #9060788 - Flags: review?(frgrahl) → review+
Comment on attachment 9060790 [details] [diff] [review]
Part 3 for cc

r+ with the items from the esr60 version addressed here too.
Attachment #9060790 - Flags: review?(frgrahl) → review+
Comment on attachment 9060794 [details] [diff] [review]
Part 5 for cc

Should this be moved to a new c-c only bug for later as long as 2.57 is not done? 

Looking thru part 1 to 3 this might be candidates for 2.53 too or is there something 2.57 specific in?
Flags: needinfo?(iann_bugzilla)

(In reply to Frank-Rainer Grahl (:frg) from comment #12)

Comment on attachment 9060783 [details] [diff] [review]
Part 1 for esr60

suite/components/pref/content/pref-applications.js
proto: HandlerInfoWrapper.prototype,

According to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
Global_Objects/Object/proto

proto is deprecated. Do we need to add it at this stage?

r/a+ with this answered.

We're moving it from the bottom to the top of the definition to match other definitions in the file, so not strictly adding it.

Flags: needinfo?(iann_bugzilla)

(In reply to Frank-Rainer Grahl (:frg) from comment #14)

Comment on attachment 9060785 [details] [diff] [review]
Part 3 for esr60

suite/components/pref/content/pref-applications.js

  • gWebContentContentConverterService: ["@mozilla.org/embeddor.implemented/web-content-handler-registrar;1", "nsIWebContentConverterService"],

Typo and should be gWebContentConverterService I assume?

r/a+ with variable fixed.

Yes, would be a better name, but I did give it the same name in all 5 locations :)

(In reply to Frank-Rainer Grahl (:frg) from comment #18)

Comment on attachment 9060794 [details] [diff] [review]
Part 5 for cc

Should this be moved to a new c-c only bug for later as long as 2.57 is not
done?

Looking thru part 1 to 3 this might be candidates for 2.53 too or is there
something 2.57 specific in?

I don't remember there being anything 2.57 specific but I'd need to check.

For part 5, there are parts that could probably be backported too but no harm them being in separate bug(s).

Carrying forward r/a+

Attachment #9060785 - Attachment is obsolete: true
Attachment #9073206 - Flags: review+
Attachment #9073206 - Flags: approval-comm-esr60+

Unbitrotted and carrying forward r+

Attachment #9060787 - Attachment is obsolete: true
Attachment #9073211 - Flags: review+

Carrying forward r+

Attachment #9060790 - Attachment is obsolete: true
Attachment #9073212 - Flags: review+

Unbitrotted, carrying forward r/a+

Attachment #9060783 - Attachment is obsolete: true
Attachment #9073216 - Flags: review+
Attachment #9073216 - Flags: approval-comm-esr60+

Updated to use Ci rather than quoted. Carrying forward r/a+

Attachment #9060784 - Attachment is obsolete: true
Attachment #9073218 - Flags: review+
Attachment #9073218 - Flags: approval-comm-esr60+

Updated to use Ci rather than quoted. Carrying forward r+

Attachment #9060788 - Attachment is obsolete: true
Attachment #9073220 - Flags: review+
Keywords: checkin-needed
Attached patch Part 1 for 2.53Splinter Review

This doesn't seem to use anything that is not in 2.53

Attachment #9073225 - Flags: review?(frgrahl)
Attachment #9073225 - Flags: approval-comm-release?
Attached patch Part 2 for 2.53Splinter Review

This doesn't seem to use anything that is not in 2.53

Attachment #9073226 - Flags: review?(frgrahl)
Attachment #9073226 - Flags: approval-comm-release?
Attached patch Part 3 for 2.53Splinter Review

PCOMUtils.defineLazyServiceGetters did not land until 57, so used multiple XPCOMUtils.defineLazyServiceGetter instead, other than that fairly 1-2-1 port from 2.57 version

Attachment #9073233 - Flags: review?(frgrahl)
Attachment #9073233 - Flags: approval-comm-release?

(In reply to Ian Neal from comment #30)

Created attachment 9073233 [details] [diff] [review]
Part 3 for 2.53

XPCOMUtils.defineLazyServiceGetters did not land until 57, so used multiple XPCOMUtils.defineLazyServiceGetter instead, other than that fairly 1-2-1 port from 2.57 version

Another option would be to backport Bug 1388215: Part 1 - Add defineLazyModuleGetters and defineLazyServiceGetters methods

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/bcf0c2684ace
Update and tidy SeaMonkey application preferences - Part 1: fixing commas, semicolons and old function naming. r=frg
https://hg.mozilla.org/comm-central/rev/60e3e0e72160
Update and tidy SeaMonkey application preferences - Part 2: general fixes up to esr60 level. r=frg
https://hg.mozilla.org/comm-central/rev/aedfbb684ce8
Update and tidy SeaMonkey application preferences - Part 3: use lazy service getters. r=frg

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED

https://hg.mozilla.org/releases/comm-esr60/rev/f57ed4b263a761119f19a7394666a524ce2720ae
Update and tidy SeaMonkey application preferences - Part 1: fixing commas, semicolons and old function naming. r=frg a=frg
https://hg.mozilla.org/releases/comm-esr60/rev/9db57638b6a0cd2adbca3f6962a030e6a546d6e0
Update and tidy SeaMonkey application preferences - Part 2: general fixes up to esr60 level. r=frg a=frg
https://hg.mozilla.org/releases/comm-esr60/rev/e78e9ce04e4ab6848f7092a56ca9bfcd39738bd8
Update and tidy SeaMonkey application preferences - Part 3: use lazy service getters. r=frg a=frg

Comment on attachment 9073225 [details] [diff] [review]
Part 1 for 2.53

Looks fine. r+ a+ Checked into gitlab for Bills 2.53 Build.
Attachment #9073225 - Flags: review?(frgrahl)
Attachment #9073225 - Flags: review+
Attachment #9073225 - Flags: approval-comm-release?
Attachment #9073225 - Flags: approval-comm-release+
Comment on attachment 9073226 [details] [diff] [review]
Part 2 for 2.53

Looks fine. r+ a+ Checked into gitlab for Bills 2.53 Build.
Attachment #9073226 - Flags: review?(frgrahl)
Attachment #9073226 - Flags: review+
Attachment #9073226 - Flags: approval-comm-release?
Attachment #9073226 - Flags: approval-comm-release+
Comment on attachment 9073233 [details] [diff] [review]
Part 3 for 2.53

Looks fine. r+ a+ Checked into gitlab for Bills 2.53 Build.
Attachment #9073233 - Flags: review?(frgrahl)
Attachment #9073233 - Flags: review+
Attachment #9073233 - Flags: approval-comm-release?
Attachment #9073233 - Flags: approval-comm-release+

Target 2.53.1
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/6922f4d683ef1cbb6d7f3f4d3fad2ae8d9c46549
Update and tidy SeaMonkey application preferences - Part 1: fixing commas, semicolons and old function naming. r=frg a=frg
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/f2db6da952c5e36083c2b07de9329bb642d84d3b
Update and tidy SeaMonkey application preferences - Part 1: fixing commas, semicolons and old function naming. r=frg a=frg
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/3173d962ff2ef89479ce1ccc3c012e7fd42002c2
Update and tidy SeaMonkey application preferences - Part 3: use lazy service getters. r=frg a=frg

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