607366 - Migration Assistant should use nsIMessenger instead of DownloadUtils for file sizes

Status: RESOLVED WONTFIX

(Thunderbird :: Migration, defect)

Description

[Jim Porter (:squib)]

Attachment: Use FormatFileSize

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

Comment 1

[Jim Porter (:squib)]

Blake, could you take a look at this? I think you were the one who originally wanted this for the Migration Assistant anyway...

Comment 2

[Blake Winton (:bwinton) (:☕️)]

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.

Comment 3

[Jim Porter (:squib)]

Attachment: 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.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Cool.  Looks good to me.

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

Later,
Blake.

Comment 5

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/6eb853899d58

Comment 6

[Mark Banner (:standard8)]

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

Comment 7

[Jim Porter (:squib)]

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.)

Comment 8

[Jim Porter (:squib)]

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.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Pushed to try as <http://hg.mozilla.org/try-comm-central/rev/8130029147c6>
You can watch it at <http://tinderbox.mozilla.org/showbuilds.cgi?tree=ThunderbirdTry> or <http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry>

When the builds are ready, they will show up at <http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/[e-mail]-8130029147c6/>

Later,
Blake.

Comment 10

[Jim Porter (:squib)]

Hm, maybe it is my fault. I guess I'll have to look at it when I have access to my dev box again...

Comment 11

[Jim Porter (:squib)]

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.)

Comment 12

[Ludovic Hirlimann [:Usul]]

Gary can you act on comment 11 ?

Comment 13

[Jim Porter (:squib)]

Attachment: Make sure sizeDifference is non-negative

This might help... could someone push this to try (just for OS X)?

Comment 14

[Jim Porter (:squib)]

As I recall, the migration assistant will be going away. Should this be WONTIFX?

Comment 15

[Ludovic Hirlimann [:Usul]]

(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 ?

Comment 16

[Jim Porter (:squib)]

Let's just WONTFIX this, since it's silly to worry about improving code that we're going to remove anyway.