Remove some deprecated code from Places

RESOLVED FIXED in mozilla11

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

(Blocks: 1 bug, {addon-compat, dev-doc-complete})

Trunk
mozilla11
addon-compat, dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox10 unaffected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
We can remove some deprecated or unused code.
(Assignee)

Comment 1

7 years ago
Created attachment 568566 [details] [diff] [review]
patch v1.0

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)
(Assignee)

Updated

7 years ago
Flags: in-testsuite-
Keywords: addon-compat
Attachment #568566 - Flags: review?(dietrich) → review+
(Assignee)

Comment 2

7 years ago
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
(Assignee)

Comment 4

7 years ago
(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+
(Assignee)

Comment 6

7 years ago
Created attachment 568864 [details] [diff] [review]
patch v1.1

addressed comment
Attachment #568566 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/1ed8be9fed39
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Assignee)

Updated

7 years ago
Keywords: dev-doc-needed
(Assignee)

Updated

7 years ago
Blocks: 629371

Updated

7 years ago
Depends on: 702081
(Assignee)

Comment 9

7 years ago
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?

Updated

7 years ago
Attachment #568864 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 10

7 years ago
backed out from Aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/7e62f3dbd056
status-firefox10: --- → unaffected
Target Milestone: mozilla10 → mozilla11
Is this going to re-land?
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 14

7 years ago
(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.