Closed
Bug 700296
Opened 9 years ago
Closed 9 years ago
Kill dynamic containers, now.
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: mak, Assigned: mak)
References
Details
(Keywords: addon-compat, dev-doc-needed, Whiteboard: [killthem])
Attachments
(3 files, 1 obsolete file)
1.23 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
15.67 KB,
patch
|
Gavin
:
superreview+
|
Details | Diff | Splinter Review |
64.04 KB,
patch
|
Details | Diff | Splinter Review |
They never worked, we tried efforts to rewrite them but never found the needed resources. They add complication and unused code paths, so I prefer just to kill them now, also because in the new schema I want to avoid their additional weight in the bookmarks table, and regardless, we need to make a new async bookmarks interface, complication may be an issue. I have a slightly alternative interface-less idea, where bookmarks service keeps a temp table and you can just put your uri nodes there and we automatically pick them up when the folder is opened, there should just be a way for the external provider to notify the folder node to update its children, shouldn't be hard. Btw, whatever we do it's better to restart from scratch here. PS: plan to do this removal in the FF11 timeframe.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #572474 -
Flags: review?(dietrich)
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 572474 [details] [diff] [review] patch v1.0 From what I remember Sync just ignores dynamic containers, I am removing the management here (see the services/ change) and I can't find further management... Flagging rnewman to ensure I'm not missing some requirement on sync side.
Attachment #572474 -
Flags: review?(rnewman)
Assignee | ||
Comment 3•9 years ago
|
||
note to self: remember to update the schema version in head_common.js!
Comment 4•9 years ago
|
||
Comment on attachment 572474 [details] [diff] [review] patch v1.0 Review of attachment 572474 [details] [diff] [review]: ----------------------------------------------------------------- r=me, looks good. please over-communicate this to Jorge on add-ons team.
Attachment #572474 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Dietrich Ayala (:dietrich) from comment #4) > r=me, looks good. please over-communicate this to Jorge on add-ons team. I'll notify in addons newgroup, btw considered they NEVER worked and we never fixed them, I'd be really surprised to find someone using them. And if they do, they are likely breaking our views.
Assignee | ||
Comment 6•9 years ago
|
||
Splitting the path to easy review. Richard, any ETA on this review? it's blocking next refactorings.
Attachment #572474 -
Attachment is obsolete: true
Attachment #572474 -
Flags: review?(rnewman)
Attachment #574637 -
Flags: review?(rnewman)
Assignee | ||
Comment 7•9 years ago
|
||
These are the interfaces changes, mostly removals. I doubt anyone ever used them, btw.
Attachment #574640 -
Flags: superreview?(robert.bugzilla)
Assignee | ||
Comment 8•9 years ago
|
||
This is the already reviewed part.
Comment 9•9 years ago
|
||
Comment on attachment 574637 [details] [diff] [review] Sync patch v1.0 Review of attachment 574637 [details] [diff] [review]: ----------------------------------------------------------------- This looks safe to me.
Attachment #574637 -
Flags: review?(rnewman) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #574640 -
Flags: superreview?(robert.bugzilla) → superreview?(gavin.sharp)
Updated•9 years ago
|
Attachment #574640 -
Flags: superreview?(gavin.sharp) → superreview+
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/177eedec0f11 https://hg.mozilla.org/integration/mozilla-inbound/rev/4be8546cdee2 https://hg.mozilla.org/integration/mozilla-inbound/rev/457b6bafbf3a I will post about the change in m.d.extensions, as soon as I'm sure it sticks.
Target Milestone: --- → mozilla11
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/177eedec0f11 https://hg.mozilla.org/mozilla-central/rev/4be8546cdee2 https://hg.mozilla.org/mozilla-central/rev/457b6bafbf3a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•9 years ago
|
Flags: in-testsuite-
Assignee | ||
Comment 12•9 years ago
|
||
posted about the removal to moz.dev.extensions
Keywords: dev-doc-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•