Closed Bug 607366 Opened 11 years ago Closed 10 years ago

Migration Assistant should use nsIMessenger instead of DownloadUtils for file sizes

Categories

(Thunderbird :: Migration, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX
Thunderbird 3.3a1

People

(Reporter: squib, Assigned: squib)

Details

(Whiteboard: qawanted)

Attachments

(2 files, 1 obsolete file)

Attached patch Use FormatFileSize (obsolete) — Splinter Review
Since bug 516787 is fixed, the Migration Assistant should use FormatFileSize from there to display sizes, instead of using DownloadUtils from Firefox.
Attachment #486092 - Flags: review?(bwinton)
Blake, could you take a look at this? I think you were the one who originally wanted this for the Migration Assistant anyway...
Assignee: nobody → squibblyflabbetydoo
Comment on attachment 486092 [details] [diff] [review]
Use FormatFileSize

It looks okay to me.  I'ld like to see a screencap of the code running, though, if you don't mind, cause we don't really have tests for this page of the migration wizard.

Thanks,
Blake.
Attachment #486092 - Flags: review?(bwinton) → review+
Here it is in action.

Also, I tried reapplying my patch and I got a patch failure, so I figured I'd been bitrotted and rewrote it. However, the new patch is identical to the old one, so I don't know what happened. If there's a problem applying the patch on your end, let me know.
Cool.  Looks good to me.

I sometimes hit the bad patch thing if I switch from Windows to Linux/Mac or back.

Later,
Blake.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/6eb853899d58
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a1
This seems to have caused MozMill test failures on Mac. I backed it out as I don't have time to investigate at the moment:

http://hg.mozilla.org/comm-central/rev/6ed4dbcb748f

TEST-UNEXPECTED-FAIL | test-1-introduction.js | test_open_and_close_migration_assistant
  EXCEPTION: a != b: 'button' != 'button syncing'.
    at: test-folder-display-helpers.js line 2535
       assert_true(false,"a != b: 'button' != 'button syncing'.") test-folder-display-helpers.js 2535
       assert_equals("button","button syncing") test-folder-display-helpers.js 2522
       check_autosync_page([object Object]) test-1-introduction.js 126
       test_open_and_close_migration_assistant() test-1-introduction.js 76
            frame.js 474
            frame.js 530
            frame.js 573
            frame.js 417
            frame.js 579
            server.js 164
            server.js 168
TEST-PASS |  teardownModule
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I get a test failure in test-1-introduction.js on Linux trunk without this patch, so chances are it's something else. (I also notice that the "required space" in the test run is -1318 bytes; with the patch, this becomes 4.0 GB, but everything else is the same for me.)
Would it be possible to get this pushed to a tryserver to see what happens? As I mentioned in comment 7, I have a suspicion that the failures were caused by something else.
Hm, maybe it is my fault. I guess I'll have to look at it when I have access to my dev box again...
Sigh... of course, it works fine for me on Linux, so I'm at a loss. Is there anyone with a Mac who can try going through migration with the builds in comment 9 to see if there's something useful in the error console? Specifically, it would be on the synchronization page.

(It's probably bad that the required space for messages on the mozmill test is negative - see comment 7 - but I'm not sure that's the actual problem.)
Gary can you act on comment 11 ?
Whiteboard: qawanted
This might help... could someone push this to try (just for OS X)?
Attachment #486092 - Attachment is obsolete: true
As I recall, the migration assistant will be going away. Should this be WONTIFX?
(In reply to Jim Porter (:squib) from comment #14)
> As I recall, the migration assistant will be going away. Should this be
> WONTIFX?

I agree how about we make that bug dependent from the removal of the assistant ?
Let's just WONTFIX this, since it's silly to worry about improving code that we're going to remove anyway.
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.