Closed Bug 961177 Opened 9 years ago Closed 9 years ago

given argument incorrect for alertDownloadSave2

Categories

(Firefox for Metro Graveyard :: Downloads, defect, P1)

Product:
Firefox for Metro Graveyard
Component:
Downloads
Type:
defect

Tracking

(firefox28 verified, firefox29 verified, firefox30 verified, b2g-v1.3 fixed)

Status:
VERIFIED FIXED
Milestone:
Firefox 30
Tracking Flags:
Tracking Status
firefox28 --- verified
firefox29 --- verified
firefox30 --- verified
b2g-v1.3 --- fixed

People

(Reporter: yfdyh000, Assigned: ally)

References

Details

(Keywords: regression, Whiteboard: p=1 s=it-30c-29a-28b.1 r=ff30)

Attachments

(2 files, 2 obsolete files)

refixDownloadSizebug961177
1.13 KB, patch
mbrubeck
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
4thFixDownloadSizebug961177.patch
1.14 KB, patch
mbrubeck
: review+
mbrubeck
: checkin+
Details | Diff | Splinter Review 
>Do you want to open or save #1 (#2) from #3?
># LOCALIZATION NOTE (alertDownloadSave2): #1 is the file name, #2 is the file size, #3 is the file host
at http://dxr.mozilla.org/mozilla-central/source/browser/metro/components/HelperAppDialog.js#124, the second 'aLauncher.suggestedFileName' should be 'aLauncher.contentLength'.
Status: UNCONFIRMED → NEW
Component: General → Downloads
Ever confirmed: true
Product: Firefox → Firefox for Metro
Blocks: 893091
Keywords: regression
Blocks: metrov1backlog
Whiteboard: [triage] [defect] p=0
Whiteboard: [triage] [defect] p=0 → [beta28] [defect] p=0
Assignee: nobody → ally
Whiteboard: [beta28] [defect] p=0 → [beta28] [defect] p=1
Blocks: metrov1it23
No longer blocks: metrov1backlog
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Attached patch incorrectArgAlertDownloadSave2 (obsolete) — — Splinter Review 

        Attachment #8363896 -
        Flags: review?(mbrubeck)

    
    

    
  
Comment on attachment 8363896 [details] [diff] [review]
incorrectArgAlertDownloadSave2

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

r=mbrubeck with a possible change to test out:

::: browser/metro/components/HelperAppDialog.js
@@ +120,5 @@
>                          text: aLauncher.suggestedFileName,
>                          className: "download-filename-text"
>                        },
>                        {
> +                        text: aLauncher.contentLength,

I think this should be "text: downloadSize," instead.  (downloadSize is a more human-readable version of contentLength.)

        Attachment #8363896 -
        Flags: review?(mbrubeck) → review+

    
    

    
  


  
Yea, I like that better. Let's go with that.

        Attachment #8363896 -
        Attachment is obsolete: true

    
  
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/integration/fx-team/rev/df864217d6d7
Keywords: checkin-needed
Whiteboard: [beta28] [defect] p=1 → [beta28] [defect] p=1[fixed-in-fx-team]

    
    

    
  
thanks RyanVM :)

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/df864217d6d7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [beta28] [defect] p=1[fixed-in-fx-team] → [beta28] [defect] p=1
Target Milestone: --- → Firefox 29

    
    

    
  
Could you please give some guidance in order for the QA to verify this? Thanks!
Flags: needinfo?(ally)

    
    

    
  
STR:
open metro, download a file
look for the notification dialog

the file size argument should be a number, not a file name
Flags: needinfo?(ally)

    
    

    
  
I seen the '(undefined)' on Nightly 29.0a1 (2014-02-03) Metro mode.

I think we should change the 'aLauncher.downloadSize' to 'downloadSize'.

    
    

    
  
re-opening to checkout and fix if need be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No longer blocks: metrov1it23
Whiteboard: [beta28] [defect] p=1 → [beta28] [defect] p=1 s=it-30c-29a-28b.1

    
    

    
  
(In reply to YF (Yang) from comment #10)
> I seen the '(undefined)' on Nightly 29.0a1 (2014-02-03) Metro mode.
> 
> I think we should change the 'aLauncher.downloadSize' to 'downloadSize'.

thank you for testing. We appreciate it. :)
Priority: P2 → P1
Whiteboard: [beta28] [defect] p=1 s=it-30c-29a-28b.1 → [release28] [defect] p=1 s=it-30c-29a-28b.1
Target Milestone: Firefox 29 → Firefox 28

    
    

    
  


  
ok, should be fixed. 

fwiw, I used the wallpapers on this site for testing: http://www.girlgeniusonline.com/fun/freestuff.php

        Attachment #8364027 -
        Attachment is obsolete: true

        Attachment #8370364 -
        Flags: review?(mbrubeck)

        Attachment #8370364 -
        Flags: review?(mbrubeck) → review+

    
  
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/integration/fx-team/rev/401da0d3ca51
Keywords: checkin-needed
Whiteboard: [release28] [defect] p=1 s=it-30c-29a-28b.1 → [release28] [defect] p=1 s=it-30c-29a-28b.1[fixed-in-fx-team]
Target Milestone: Firefox 28 → ---

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/401da0d3ca51
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Whiteboard: [release28] [defect] p=1 s=it-30c-29a-28b.1[fixed-in-fx-team] → [release28] [defect] p=1 s=it-30c-29a-28b.1
Target Milestone: --- → Firefox 30

    
    

    
  
I still seen the '(undefined)'.
Build: Mozilla/5.0 (Windows NT 6.3; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140206030201 CSet: 1e9f169c9715

Please add 'let ' to 'downloadSize = this._getDownloadSize(aLauncher.contentLength);'.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  


  
This is my first patch for Mozilla hg. :)

        Attachment #8372215 -
        Flags: review?(mbrubeck)

    
  
Keywords: checkin-needed
Status: REOPENED → ASSIGNED

    
    

    
  
Please wait until you have r+ before requesting checkin.
Keywords: checkin-needed

    
    

    
  
Comment on attachment 8372215 [details] [diff] [review]
4thFixDownloadSizebug961177.patch

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

Thank you!  I'll check this in the next time I push to fx-team.

        Attachment #8372215 -
        Flags: review?(mbrubeck)

        Attachment #8372215 -
        Flags: review+

        Attachment #8372215 -
        Flags: checkin?(mbrubeck)

    
    

    
  
Comment on attachment 8372215 [details] [diff] [review]
4thFixDownloadSizebug961177.patch

https://hg.mozilla.org/integration/fx-team/rev/84cc01e1c621

Not requesting approval on this patch because it is code cleanup only, not required to fix the bug.


(In reply to YF (Yang) from comment #16)
> I still seen the '(undefined)'.
> Build: Mozilla/5.0 (Windows NT 6.3; rv:30.0) Gecko/20100101 Firefox/30.0
> ID:20140206030201 CSet: 1e9f169c9715

This was built before attachment 8370364 [details] [diff] [review] landed.  The bug fix is in today's nightly build (2004-02-07).

        Attachment #8372215 -
        Flags: checkin?(mbrubeck) → checkin+

    
    

    
  
Comment on attachment 8370364 [details] [diff] [review]
refixDownloadSizebug961177

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 893091

User impact if declined: "undefined" is displayed instead of file size, in Metro download toolbar.

Testing completed (on m-c, etc.): Landed on m-c on 2014-02-06.

Risk to taking this patch (and alternatives if risky): Very low-risk, trivial one-line patch.  Metro-only.

String or IDL/UUID changes made by this patch: None.

        Attachment #8370364 -
        Flags: approval-mozilla-beta?

        Attachment #8370364 -
        Flags: approval-mozilla-aurora?

    
    

    
  
(In reply to Matt Brubeck (:mbrubeck) from comment #20)
> Comment on attachment 8372215 [details] [diff] [review]
> 4thFixDownloadSizebug961177.patch
> 
> https://hg.mozilla.org/integration/fx-team/rev/84cc01e1c621
> 
> Not requesting approval on this patch because it is code cleanup only, not
> required to fix the bug.
> 
> 
> (In reply to YF (Yang) from comment #16)
> > I still seen the '(undefined)'.
> > Build: Mozilla/5.0 (Windows NT 6.3; rv:30.0) Gecko/20100101 Firefox/30.0
> > ID:20140206030201 CSet: 1e9f169c9715
> 
> This was built before attachment 8370364 [details] [diff] [review] landed. 
> The bug fix is in today's nightly build (2004-02-07).

Thanks for testing. I just found I misread the changeset log before, mistakenly believe the '20140206' already contains the 'refixDownloadSizebug961177' patch.

It work on Nightly 20140207 now.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/84cc01e1c621
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Whiteboard: [release28] [defect] p=1 s=it-30c-29a-28b.1 → p=1 s=it-30c-29a-28b.1 r=ff30
Blocks: metrobacklog
No longer blocks: metrov1backlog

        Attachment #8370364 -
        Flags: approval-mozilla-beta?

        Attachment #8370364 -
        Flags: approval-mozilla-beta+

        Attachment #8370364 -
        Flags: approval-mozilla-aurora?

        Attachment #8370364 -
        Flags: approval-mozilla-aurora+

    
  
Keywords: verifyme

    
    

    
  
Verified as fixed with latest Nightly, Aurora and Beta (28 beta 2) on Win 8 64-bit: while downloading, the file size argument is a number.
Status: RESOLVED → VERIFIED

          status-firefox28:
          fixed → verified

          status-firefox29:
          fixed → verified

          status-firefox30:
          fixed → verified
Keywords: verifyme

          You need to log in
          before you can comment on or make changes to this bug.