Closed Bug 696159 Opened 9 years ago Closed 9 years ago

Remove some deprecated code from Places

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11
Tracking Status
firefox10 --- unaffected

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: addon-compat, dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

We can remove some deprecated or unused code.
Attached patch patch v1.0 (obsolete) — Splinter Review
I was hoping to remove more, we have far more useless stuff, but that would be too much time consuming atm, so for now let's just remove this stuff, deprecated from quite some releases.
I did not touch old GUID APIs since XMarks is still using them and we are not yet providing alternatives (unless they manually query the db or base the addon on notifications, sync does the latter). It's likely they need a rewrite, should be contacted.
I'll then try to file bugs for the various removals we could do and tag them with some whiteboard entry.
Attachment #568566 - Flags: review?(dietrich)
Flags: in-testsuite-
Keywords: addon-compat
Attachment #568566 - Flags: review?(dietrich) → review+
Comment on attachment 568566 [details] [diff] [review]
patch v1.0

asking SR for the deprecated method removal from the results interface.
Attachment #568566 - Flags: superreview?(robert.bugzilla)
Comment on attachment 568566 [details] [diff] [review]
patch v1.0

I'm a tad buried right now and I'm hoping you could provide the reason for the change to removed-files.in so I don't have to figure it out. ;)

>diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in
>--- a/browser/installer/removed-files.in
>+++ b/browser/installer/removed-files.in
>@@ -992,17 +992,16 @@ xpicleanup@BIN_SUFFIX@
>   modules/services-sync/type_records/passwords.js
>   modules/services-sync/type_records/prefs.js
>   modules/services-sync/type_records/tabs.js
>   modules/services-sync/util.js
>   modules/stylePanel.jsm
>   modules/tabview/AllTabs.jsm
>   modules/tabview/groups.jsm
>   modules/tabview/utils.jsm
>-  modules/utils.js
>   modules/WindowDraggingUtils.jsm
>   #ifdef XP_WIN
>     modules/WindowsJumpLists.jsm
>     modules/WindowsPreviewPerTab.jsm
>   #endif
>   modules/XPCOMUtils.jsm
>   modules/XPIProvider.jsm
>   res/contenteditable.css
>@@ -1385,8 +1384,9 @@ extensions/inspector@mozilla.org/chrome/
> extensions/inspector@mozilla.org/defaults/preferences/inspector.js
> extensions/inspector@mozilla.org/platform/WINNT/chrome/icons/default/winInspectorMain.ico
> extensions/inspector@mozilla.org/components/inspector.xpt
> extensions/inspector@mozilla.org/components/@DLL_PREFIX@inspector@DLL_SUFFIX@
> extensions/inspector@mozilla.org/chrome/icons/default/winInspectorMain.ico
> components/nsPlacesTransactionsService.js
> components/browserplaces.xpt
> components/nsPlacesDBFlush.js
>+modules/utils.js
(In reply to Robert Strong [:rstrong] (do not email) from comment #3)
> Comment on attachment 568566 [details] [diff] [review] [diff] [details] [review]
> patch v1.0
> 
> I'm a tad buried right now and I'm hoping you could provide the reason for
> the change to removed-files.in so I don't have to figure it out. ;)

utils.js is a compatibility mock we left in for compatibility reasons, it just forwards to PlacesUtils.jsm. So I would like to remove it after 6 releases. I moved it from the ifdef OMNIJAR to the global position, since it should be removed in both cases.
Comment on attachment 568566 [details] [diff] [review]
patch v1.0

Please add it after modules/JSON.jsm instead of at the end of removed-files.in
  modules/JSON.jsm
+ modules/utils.js
Attachment #568566 - Flags: superreview?(robert.bugzilla) → superreview+
Attached patch patch v1.1Splinter Review
addressed comment
Attachment #568566 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/1ed8be9fed39
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Keywords: dev-doc-needed
Blocks: 629371
Depends on: 702081
Comment on attachment 568864 [details] [diff] [review]
patch v1.1

This caused regressions that make harder to use the bookmarks dialogs, I'm fixing those in bug 702081, but I'd want to backout this from Aurora since I don't want to backport the fixes to limit the risk.

So, asking approval for backout.
Attachment #568864 - Flags: approval-mozilla-aurora?
Attachment #568864 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
backed out from Aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/7e62f3dbd056
Target Milestone: mozilla10 → mozilla11
Is this going to re-land?
(In reply to Eric Shepherd [:sheppy] from comment #11)
> Is this going to re-land?

not on mozilla 10 obviously, though it's fixed from 11 on.
You need to log in before you can comment on or make changes to this bug.