1081101 - Implement eraseEverything in Bookmarks.jsm

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

Bug 1068009 implements the single removal option, batch removals must still be implemented.

Comment 1

[Marco Bonardo [:mak]]

instead of batch removals we will implement an eraseEverything API that is more coherent with our use cases (empty the roots before a restore mostly, plus tests)

Comment 2

[Marco Bonardo [:mak]]

Attachment: patch v1

I'm not totally sold on the API name, if you have better ideas, I'm listening. Alternatives to erase could be wipe or empty... alternatives to Everything may be All (but sounds less scary), Store (but it's not very clear) or Roots (but then we must introduce the roots concept in the API)

Comment 3

[Marco Bonardo [:mak]]

Attachment: patch v2

oops I forgot tpo hg add the test

Comment 4

[Asaf Romano (gone)]

Comment on attachment 8506237 [details] [diff] [review]
patch v2

Review of attachment 8506237 [details] [diff] [review]:
-----------------------------------------------------------------

Again, at this point I prefer to review one giant patch, but this patch alone looks fine.

::: toolkit/components/places/Bookmarks.jsm
@@ +890,5 @@
> +      if (val !== null) {
> +        Object.defineProperty(item, prop, { value: val
> +                                          , writable: true
> +                                          , enumerable: false
> +                                          , configurable: false });

See my comments on the main bug.

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8506237 [details] [diff] [review]
patch v2

patch merged into bug Bug 1068009.

Comment 6

[Marco Bonardo [:mak]]

fixed by Bug 1068009.