Merge nsIPrefBranch2 with nsIPrefBranch

RESOLVED FIXED in mozilla13

Status

()

Core
Preferences: Backend
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

({addon-compat, dev-doc-complete})

Trunk
mozilla13
addon-compat, dev-doc-complete
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
It's time to get rid of nsIPrefBranch2. I'll merge it with nsIPrefBranch and leave an empty interface for compatibility.

Benjamin, are you willing to do reviews for this?
(Assignee)

Comment 1

6 years ago
Created attachment 590145 [details] [diff] [review]
part A, merge interfaces
Attachment #590145 - Flags: review?(benjamin)
(Assignee)

Comment 2

6 years ago
Created attachment 590146 [details] [diff] [review]
part B, modules/
Attachment #590146 - Flags: review?(benjamin)
(Assignee)

Comment 3

6 years ago
Created attachment 590148 [details] [diff] [review]
part C, toolkit/
Attachment #590148 - Flags: review?(benjamin)
(Assignee)

Comment 4

6 years ago
Created attachment 590149 [details] [diff] [review]
part D, browser/ and mobile/
Attachment #590149 - Flags: review?(benjamin)
(Assignee)

Comment 5

6 years ago
Created attachment 590150 [details] [diff] [review]
part E, netwerk/
Attachment #590150 - Flags: review?(benjamin)
(Assignee)

Comment 6

6 years ago
Created attachment 590151 [details] [diff] [review]
part F, testing/ and layout/tools/reftest/
Attachment #590151 - Flags: review?(benjamin)
(Assignee)

Comment 7

6 years ago
Created attachment 590153 [details] [diff] [review]
part G, everything else
Attachment #590153 - Flags: review?(benjamin)

Comment 8

6 years ago
I will get to this review on Thurdsay, I'm traveling this week.

Updated

6 years ago
Attachment #590145 - Flags: review?(benjamin) → review+
Just to be sure: all these patches build on their own?

Updated

6 years ago
Attachment #590146 - Flags: review?(benjamin) → review+

Updated

6 years ago
Attachment #590148 - Flags: review?(benjamin) → review+

Updated

6 years ago
Attachment #590149 - Flags: review?(benjamin) → review+

Updated

6 years ago
Attachment #590150 - Flags: review?(benjamin) → review+

Updated

6 years ago
Attachment #590151 - Flags: review?(benjamin) → review+

Updated

6 years ago
Attachment #590153 - Flags: review?(benjamin) → review+
(Assignee)

Comment 10

6 years ago
Comment on attachment 590145 [details] [diff] [review]
part A, merge interfaces

(In reply to Ms2ger from comment #9)
> Just to be sure: all these patches build on their own?

I sent it all to TryServer every time I started a new part. Only A and B actually change how things work.
Attachment #590145 - Flags: superreview?(dwitte)
Attachment #590145 - Flags: superreview?(dwitte) → superreview+
(Assignee)

Comment 11

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e933d31eb696
https://hg.mozilla.org/integration/mozilla-inbound/rev/910cc1ad2803
https://hg.mozilla.org/integration/mozilla-inbound/rev/860513cc7d5a
https://hg.mozilla.org/integration/mozilla-inbound/rev/102dd9765964
https://hg.mozilla.org/integration/mozilla-inbound/rev/07800b5b4d83
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc98d884f0f0
https://hg.mozilla.org/integration/mozilla-inbound/rev/04ac18d14436

We should mark nsIPrefBranch2 as deprecated on MDN and in the addon validator.
Flags: in-testsuite-
Flags: in-litmus-
Keywords: addon-compat, dev-doc-needed
Target Milestone: --- → mozilla13
(Assignee)

Comment 12

6 years ago
I'll need to follow this up with some new additions. Will do so after a few days in case there's stuff from other branches.
Whiteboard: [leave open after inbound merge]
https://hg.mozilla.org/mozilla-central/rev/e933d31eb696
https://hg.mozilla.org/mozilla-central/rev/910cc1ad2803
https://hg.mozilla.org/mozilla-central/rev/860513cc7d5a
https://hg.mozilla.org/mozilla-central/rev/102dd9765964
https://hg.mozilla.org/mozilla-central/rev/07800b5b4d83
https://hg.mozilla.org/mozilla-central/rev/fc98d884f0f0
https://hg.mozilla.org/mozilla-central/rev/04ac18d14436
It would be nice if a compatibility warning were emitted the first time an attempt is made to QI a preference branch to nsIPrefBranch2 so code that relies on it doesn't suddenly break without warning when it goes away.
(Assignee)

Updated

6 years ago
Blocks: 727408

Comment 15

6 years ago
kmag, I don't think we have plans to remove nsIPrefBranch2 any time soon, so I don't think that the in-product compatibility warning is needed currently.
(Assignee)

Comment 16

6 years ago
Created attachment 600535 [details] [diff] [review]
part H

Last one (hopefully)
Attachment #600535 - Flags: review?(benjamin)
Comment on attachment 600535 [details] [diff] [review]
part H

>diff --git a/toolkit/components/places/nsPlacesAutoComplete.js b/toolkit/components/places/nsPlacesAutoComplete.js

>-    if (aRegisterObserver) {
>-      let pb = this._prefs.QueryInterface(Ci.nsIPrefBranch2);
>-      pb.addObserver("", this, false);
>-    }
>+    this._prefs.addObserver("", this, false);

Don't you need to preserve the aRegisterObserver check?
Attachment #600535 - Flags: review?(benjamin) → review-
(Assignee)

Comment 18

6 years ago
Created attachment 600613 [details] [diff] [review]
part H

Oh, uhh, whoops.
Attachment #600535 - Attachment is obsolete: true
Attachment #600613 - Flags: review?(gavin.sharp)
Attachment #600613 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 19

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8a1e874238a
Whiteboard: [leave open after inbound merge]
https://hg.mozilla.org/mozilla-central/rev/a8a1e874238a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 21

6 years ago
I've updated these MDN pages. There's still a lot of mentions, but it would be too messy to change them at this point.

https://developer.mozilla.org/en/Code_snippets/Preferences
https://developer.mozilla.org/en/Firefox_13_for_developers
https://developer.mozilla.org/en/Preferences_API
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIPrefBranch
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIPrefBranch2
Keywords: dev-doc-needed
Keywords: dev-doc-complete
Comment hidden (spam)
You need to log in before you can comment on or make changes to this bug.