561231 - Create a proper b-c test for copy/paste bookmarks

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Marco Bonardo [:mak]]

We have a b-c test for copy in tags, we should have one more generic one selecting copying and pasting at least a couple bookmarks a separator and a folder to a new position

Comment 1

[Mark Banner (:standard8)]

I think we have most of this already now. The only thing missing that I realised from code coverage is support for copying a separator from a separate instance, as well as doing an actual copy of bookmarks.

I already wrote a test for those in bug 1451352, but we closed it, and I forgot about landing those. So lets use this to complete the test coverage there.

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 561231 - Add a test for copying a separator from another instance, and also copying bookmarks within the same instance.

Review commit: https://reviewboard.mozilla.org/r/234982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/234982/

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8966233 [details]
Bug 561231 - Add a test for copying a separator from another instance, and also copying bookmarks within the same instance.

https://reviewboard.mozilla.org/r/234982/#review241052

thanks, r=me with a green Try

::: browser/components/places/tests/browser/browser_paste_bookmarks.js:265
(Diff revision 1)
> +    "type": PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR
> +  };
> +  data = JSON.stringify(data);
> +
> +  xferable.addDataFlavor(PlacesUtils.TYPE_X_MOZ_PLACE);
> +  xferable.setTransferData(PlacesUtils.TYPE_X_MOZ_PLACE, PlacesUtils.toISupportsString(data),

nit: indentation

::: browser/components/places/tests/browser/browser_paste_bookmarks.js:286
(Diff revision 1)
> +               "Should have the correct type");
> +
> +  await PlacesUtils.bookmarks.remove(tree.children[0].guid);
> +});
> +
> +// XXX fixme

hm?

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8966233 [details]
Bug 561231 - Add a test for copying a separator from another instance, and also copying bookmarks within the same instance.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/234982/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2961729885f5
Add a test for copying a separator from another instance, and also copying bookmarks within the same instance. r=mak

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/2961729885f5