Closed Bug 968177 Opened 6 years ago Closed 6 years ago

Speed up bookmarks.html export by using the shared backups code

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file, 4 obsolete files)

We can reuse the code I'm introducing in bug 824433 for bookmarks.html, this will have other advantages like exporting guids and tags in the html format and using OS.File, with a relatively low cost (since the shared code is already there)
Attached patch wip (obsolete) — Splinter Review
Blocks: 423197
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #8371017 - Attachment is obsolete: true
Attachment #8372248 - Flags: review?(mano)
note I verified that IE can properly import the generated file
Blocks: 945120
note to self: remember to remove pre-processing from BookmarksHTMLUtils.jsm
Attached patch patch v1.1 (obsolete) — Splinter Review
removed preprocessing
Attachment #8372248 - Attachment is obsolete: true
Attachment #8372248 - Flags: review?(mano)
Attachment #8373347 - Flags: review?(mano)
Comment on attachment 8373347 [details] [diff] [review]
patch v1.1

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

Looks good.

::: toolkit/components/places/BookmarkHTMLUtils.jsm
@@ +115,5 @@
> +
> +/**
> + * Gets [faviconURI, dataLen, data, mimeType] for a given page url.
> + */
> +function promiseFaviconData(aPageUrl) {

It's probably a good idea to have it in PlacesUtils alongside similar methods

@@ +164,5 @@
>    /**
>     * Saves the current bookmarks hierarchy to a "bookmarks.html" file.
>     *
> +   * @param aFilePath
> +   *        OS.File path for the "bookmarks.html" file to be created.

Since it's just a file path string, I wouldn't mention OS.File.
Attachment #8373347 - Flags: review?(mano) → review+
Attached patch patch v1.2 (obsolete) — Splinter Review
Attachment #8373347 - Attachment is obsolete: true
Attached patch patch v1.3Splinter Review
correct changes :/
Attachment #8384602 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/e89b4d98aa53
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 991668
Depends on: 997030
You need to log in before you can comment on or make changes to this bug.