Merge nsIPrefBranch2 with nsIPrefBranch

RESOLVED FIXED in mozilla13

Status

()

defect
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

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

Trunk
mozilla13
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 1 obsolete attachment)

(Assignee)

Description

7 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

7 years ago
Attachment #590145 - Flags: review?(benjamin)
(Assignee)

Comment 2

7 years ago
Attachment #590146 - Flags: review?(benjamin)
(Assignee)

Comment 3

7 years ago
Attachment #590148 - Flags: review?(benjamin)
(Assignee)

Comment 4

7 years ago
Attachment #590149 - Flags: review?(benjamin)
(Assignee)

Comment 5

7 years ago
Attachment #590150 - Flags: review?(benjamin)
(Assignee)

Comment 7

7 years ago
Attachment #590153 - Flags: review?(benjamin)

Comment 8

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Comment 10

7 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 12

7 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]
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

7 years ago
Blocks: 727408
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

7 years ago
Posted patch part H (obsolete) — Splinter Review
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

7 years ago
Posted patch part HSplinter Review
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

7 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: 7 years ago
Resolution: --- → FIXED
Comment hidden (spam)
You need to log in before you can comment on or make changes to this bug.