Closed Bug 961177 Opened 11 years ago Closed 11 years ago

given argument incorrect for alertDownloadSave2

Categories

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

defect

Tracking

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

VERIFIED FIXED
Firefox 30
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)

>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
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
Keywords: checkin-needed
Whiteboard: [beta28] [defect] p=1 → [beta28] [defect] p=1[fixed-in-fx-team]
thanks RyanVM :)
Status: ASSIGNED → RESOLVED
Closed: 11 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
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 → ---
Status: REOPENED → RESOLVED
Closed: 11 years ago11 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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: