Closed Bug 1373610 Opened 3 years ago Closed 2 years ago
JSONUtils .import From JSON more robust for invalid/unknown bookmark entries
59 bytes, text/x-review-board-request
From bug 1095426, currently translateTreeTypes will throw if it finds a type that it doesn't understand. This will cause the whole import to be aborted. We should improve this, possibly by throwing and remove the node in the caller code (that would require changing from for..of to indexing and fixing the looping index).
In bug 1404631 I'll make insertTree a bit more lenient, then we can see if there's more work to do on the importers side here.
Depends on: 1404631
Looks like the code is just invoking reportError, not throwing. Apart from removing the comment here http://searchfox.org/mozilla-central/rev/40e8eb46609dcb8780764774ec550afff1eed3a5/toolkit/components/places/BookmarkJSONUtils.jsm#445 what's left to do?
(In reply to Marco Bonardo [::mak] from comment #2) > Looks like the code is just invoking reportError, not throwing. That's correct, however, if I'm reading it write, it will still continue processing the node. When it then hits insertTree, the node type would presumably be invalid, and insertTree would throw. However, I think your recent changes in bug 1404631, would mean we'll just skip the node. > Apart from > removing the comment here > http://searchfox.org/mozilla-central/rev/ > 40e8eb46609dcb8780764774ec550afff1eed3a5/toolkit/components/places/ > BookmarkJSONUtils.jsm#445 what's left to do? We might just want to change it to something like "this is an invalid node that will be skipped by insertTree", or find some way of not processing it?
Comment on attachment 8930546 [details] Bug 1373610 - Clarify BookmarkJSONUtils' handling of invalid bookmark types and add a test. https://reviewboard.mozilla.org/r/201676/#review207450 thanks
Attachment #8930546 - Flags: review?(mak77) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/d9bbfb2a7e42 Clarify BookmarkJSONUtils' handling of invalid bookmark types and add a test. r=mak
You need to log in before you can comment on or make changes to this bug.