Migration Assistant should use nsIMessenger instead of DownloadUtils for file sizes

RESOLVED WONTFIX

Status

Thunderbird
Migration
RESOLVED WONTFIX
8 years ago
7 years ago

People

(Reporter: squib, Assigned: squib)

Tracking

Trunk
Thunderbird 3.3a1
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: qawanted)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Created attachment 486092 [details] [diff] [review]
Use FormatFileSize

Since bug 516787 is fixed, the Migration Assistant should use FormatFileSize from there to display sizes, instead of using DownloadUtils from Firefox.
(Assignee)

Updated

8 years ago
Attachment #486092 - Flags: review?(bwinton)
(Assignee)

Comment 1

8 years ago
Blake, could you take a look at this? I think you were the one who originally wanted this for the Migration Assistant anyway...
(Assignee)

Updated

8 years ago
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+
(Assignee)

Comment 3

8 years ago
Created attachment 486153 [details]
The patch in action (not that you can tell!)

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
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/6eb853899d58
Status: ASSIGNED → RESOLVED
Last Resolved: 8 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 → ---
(Assignee)

Comment 7

8 years ago
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.)
(Assignee)

Comment 8

8 years ago
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.
(Assignee)

Comment 10

8 years ago
Hm, maybe it is my fault. I guess I'll have to look at it when I have access to my dev box again...
(Assignee)

Comment 11

8 years ago
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
(Assignee)

Comment 13

8 years ago
Created attachment 507676 [details] [diff] [review]
Make sure sizeDifference is non-negative

This might help... could someone push this to try (just for OS X)?
Attachment #486092 - Attachment is obsolete: true
(Assignee)

Comment 14

7 years ago
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 ?
(Assignee)

Comment 16

7 years ago
Let's just WONTFIX this, since it's silly to worry about improving code that we're going to remove anyway.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.