Merge nsIPrefBranch2 with nsIPrefBranch

RESOLVED FIXED in mozilla13

Status

()

defect
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

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

Trunk
mozilla13
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 1 obsolete attachment)

Assignee

Description

8 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.