Closed
Bug 961177
Opened 11 years ago
Closed 11 years ago
given argument incorrect for alertDownloadSave2
Categories
(Firefox for Metro Graveyard :: Downloads, defect, P1)
Firefox for Metro Graveyard
Downloads
Tracking
(firefox28 verified, firefox29 verified, firefox30 verified, b2g-v1.3 fixed)
VERIFIED
FIXED
Firefox 30
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)
1.13 KB,
patch
|
mbrubeck
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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'.
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Component: General → Downloads
Ever confirmed: true
Product: Firefox → Firefox for Metro
Updated•11 years ago
|
Blocks: 893091
Keywords: regression
Updated•11 years ago
|
Blocks: metrov1backlog
Whiteboard: [triage] [defect] p=0
Updated•11 years ago
|
Whiteboard: [triage] [defect] p=0 → [beta28] [defect] p=0
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → ally
Updated•11 years ago
|
Whiteboard: [beta28] [defect] p=0 → [beta28] [defect] p=1
Updated•11 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #8363896 -
Flags: review?(mbrubeck)
Comment 2•11 years ago
|
||
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•11 years ago
|
||
Yea, I like that better. Let's go with that.
Attachment #8363896 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 4•11 years ago
|
||
Keywords: checkin-needed
Whiteboard: [beta28] [defect] p=1 → [beta28] [defect] p=1[fixed-in-fx-team]
Assignee | ||
Comment 5•11 years ago
|
||
thanks RyanVM :)
Comment 6•11 years ago
|
||
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
Comment 7•11 years ago
|
||
status-firefox28:
--- → fixed
status-firefox29:
--- → fixed
Comment 8•11 years ago
|
||
Could you please give some guidance in order for the QA to verify this? Thanks!
Flags: needinfo?(ally)
Assignee | ||
Comment 9•11 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•11 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•11 years ago
|
||
re-opening to checkout and fix if need be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•11 years ago
|
status-firefox28:
fixed → ---
status-firefox29:
fixed → ---
Updated•11 years ago
|
No longer blocks: metrov1it23
Whiteboard: [beta28] [defect] p=1 → [beta28] [defect] p=1 s=it-30c-29a-28b.1
Assignee | ||
Comment 12•11 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. :)
Updated•11 years ago
|
Priority: P2 → P1
Whiteboard: [beta28] [defect] p=1 s=it-30c-29a-28b.1 → [release28] [defect] p=1 s=it-30c-29a-28b.1
Updated•11 years ago
|
Target Milestone: Firefox 29 → Firefox 28
Updated•11 years ago
|
Blocks: metrov1backlog
Assignee | ||
Comment 13•11 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)
Updated•11 years ago
|
Attachment #8370364 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 14•11 years ago
|
||
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]
Updated•11 years ago
|
Target Milestone: Firefox 28 → ---
Comment 15•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 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•11 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•11 years ago
|
||
This is my first patch for Mozilla hg. :)
Attachment #8372215 -
Flags: review?(mbrubeck)
Keywords: checkin-needed
Updated•11 years ago
|
Status: REOPENED → ASSIGNED
Comment 18•11 years ago
|
||
Please wait until you have r+ before requesting checkin.
Keywords: checkin-needed
Comment 19•11 years ago
|
||
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 20•11 years ago
|
||
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 21•11 years ago
|
||
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•11 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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [release28] [defect] p=1 s=it-30c-29a-28b.1 → p=1 s=it-30c-29a-28b.1 r=ff30
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #8370364 -
Flags: approval-mozilla-beta?
Attachment #8370364 -
Flags: approval-mozilla-beta+
Attachment #8370364 -
Flags: approval-mozilla-aurora?
Attachment #8370364 -
Flags: approval-mozilla-aurora+
Comment 24•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/9e81f20df0b0
https://hg.mozilla.org/releases/mozilla-beta/rev/9cb138198688
status-firefox28:
--- → fixed
status-firefox29:
--- → fixed
Updated•11 years ago
|
status-firefox30:
--- → fixed
Comment 25•11 years ago
|
||
Updated•11 years ago
|
status-b2g-v1.3:
--- → fixed
Comment 26•11 years ago
|
||
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
Keywords: verifyme
You need to log in
before you can comment on or make changes to this bug.
Description
•