Allow import of bookmarks from other profiles that we support migration from.

RESOLVED FIXED

Status

defect
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

(Blocks 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

12 years ago
I'd already implemented this in some early patches on bug 329744. Now that's landed we can prepare this to land at switchover time for suiterunner.
Assignee

Comment 1

12 years ago
Posted patch Possible patch (obsolete) — Splinter Review
This works, but when selecting from file, you can see the migrator wizard advancing to the next page before the file open dialog comes up.

I'm not sure why this happens yet, it seems like these lines aren't doing their job:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/profile/migration/migration.js&rev=1.1&mark=154-155#147
Assignee

Comment 2

12 years ago
Posted patch Patch v2 (obsolete) — Splinter Review
Allow import of bookmarks from existing profiles/apps (where the profile migrator supports them).

The option is available under tools->import of the bookmarks manager.

Note that I've added some bits to stop the wizard advancing to the next page when the from file option is chosen in case the file dialog is slow to come up and the wizard is slow to cancel (as seen on my system), this also prevents an error occurring due to the next page trying to use something that isn't defined because we've selected to import from a file.

Requesting approvals now so we're ready to land this at or soon after the suiterunner switch on trunk.
Attachment #265427 - Attachment is obsolete: true
Attachment #265439 - Flags: superreview?(neil)
Attachment #265439 - Flags: review?(neil)
Comment on attachment 265439 [details] [diff] [review]
Patch v2

>+#ifdef XP_MACOSX
>+    var features = "centerscreen,chrome,resizable=no";
This isn't going to work. The wizard has to be modal, otherwise we won't know whether the "File" import source has been chosen.

>+      // Don't let the wizard migrate to the next page event though we've
>+      // called cancel - cancel may not get processed first.
Odd, because it tries very hard to close the window!
Comment on attachment 265439 [details] [diff] [review]
Patch v2

So, for some reason, it's offering the choice of importing from Thunderbird, despite my never having used it, but not IE :-\
Assignee

Comment 5

12 years ago
(In reply to comment #4)
> (From update of attachment 265439 [details] [diff] [review])
> So, for some reason, it's offering the choice of importing from Thunderbird,
> despite my never having used it, but not IE :-\

It won't offer IE because we haven't written the profile migrator for it yet.

Note sure why its offering Thunderbird - I had Thunderbird profiles available for migration on linux, but it didn't offer them in the bookmarks manager. So either its a windows bug, or its some incorrect logic.
Assignee

Comment 6

12 years ago
(In reply to comment #3)
> (From update of attachment 265439 [details] [diff] [review])
> >+      // Don't let the wizard migrate to the next page event though we've
> >+      // called cancel - cancel may not get processed first.
> Odd, because it tries very hard to close the window!

Yep, but for some reason my linux build definitely showed it moving on first and generating an error before getting round to dealing with the cancel.
Assignee

Comment 7

12 years ago
Comment on attachment 265439 [details] [diff] [review]
Patch v2

New patch coming up later that removes the ifdef.
Attachment #265439 - Attachment is obsolete: true
Attachment #265439 - Flags: superreview?(neil)
Attachment #265439 - Flags: review?(neil)
Assignee

Comment 8

12 years ago
Posted patch Patch v3Splinter Review
Neil said on irc that the thunderbird problem went away after a rebuild.

This patch removes the ifdef that was previously there as it won't do any good as per Neil's comment 3.
Attachment #265545 - Flags: superreview?(neil)
Attachment #265545 - Flags: review?(neil)
Comment on attachment 265545 [details] [diff] [review]
Patch v3

>+    window.openDialog("chrome://communicator/content/migration/migration.xul",
>+                      "migration", "modal,centerscreen,chrome,resizable=no",
>+                      "bookmarks");
Sorry, I should have mentioned before that I don't like window names, that is to say, the second parameter to openDialog should always be the empty string.
Attachment #265545 - Flags: superreview?(neil)
Attachment #265545 - Flags: superreview+
Attachment #265545 - Flags: review?(neil)
Attachment #265545 - Flags: review+
Assignee

Comment 10

12 years ago
Patch checked in (with changed window title) -> fixed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.