given argument incorrect for alertDownloadSave2

VERIFIED FIXED in Firefox 28

Status

defect
P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: yfdyh000, Assigned: ally)

Tracking

({regression})

Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
>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
Whiteboard: [triage] [defect] p=0
Whiteboard: [triage] [defect] p=0 → [beta28] [defect] p=0
(Assignee)

Updated

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

Comment 1

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

Comment 3

5 years ago
Yea, I like that better. Let's go with that.
Attachment #8363896 - Attachment is obsolete: true
(Assignee)

Updated

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

Comment 5

5 years ago
thanks RyanVM :)
https://hg.mozilla.org/mozilla-central/rev/df864217d6d7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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)
(Assignee)

Comment 9

5 years ago
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)
(Reporter)

Comment 10

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

I think we should change the 'aLauncher.downloadSize' to 'downloadSize'.
(Assignee)

Comment 11

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

Comment 12

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

Comment 13

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

Updated

5 years ago
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
Last Resolved: 5 years ago5 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
(Reporter)

Comment 16

5 years ago
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 → ---
(Reporter)

Comment 17

5 years ago
This is my first patch for Mozilla hg. :)
Attachment #8372215 - Flags: review?(mbrubeck)
(Reporter)

Updated

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

Comment 22

5 years ago
(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
Last Resolved: 5 years ago5 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+

Updated

5 years ago
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.