Closed Bug 67214 Opened 23 years ago Closed 20 years ago
Int() used instead of Math .round() or Math .floor() in embedding/components/ui/progress Dlg/ns Progress Dlg .js
parseInt() is used all over the place in xpfe/components/ucth/resources/helperAppDldProgress.js. This is a function that takes a string and converts it to an integer, but it's being applied to numeric arguments! I am guessing this ends up doing a number->string conversion and then a string->integer conversion. In most cases, we are doing parseInt(foo+0.5), which is exactly equivalent to Math.round(foo), for positive foo. Since all the values in question are positive (times, completion percentages, amounts downloaded, and such), it might make sense to use Math.round().
I stumpled on this bug today. This seems to be a good candidate to speed up progress-dialogs, like the one used in downloading. The original file doesn't exists anymore, but the code seems to be moved to /embedding/components/ui/progressDlg/nsProgressDlg.js
Quick test case reveals that round and floor are more than twice as fast than parseInt. This difference might not be noticable when only done once, but when being called repeatedly it could show up. 50000 parseInt(2) take: 1010 50000 floor(2) take: 412 50000 round(2) take: 403 50000 parseInt(2.7) take: 1010 50000 floor(2.7) take: 403 50000 round(2.7) take: 415
This patch fixes the parseInt's in the progress listener of the download manager. I have run mozilla it and it works fine for me.
This changes rounds that really need to be floors to actually be floors. Ready fro review and checkin now
Comment on attachment 118751 [details] [diff] [review] v.02 Switching reviewer & adding super-reviewer in hopes of getting this fixed since bug 199655 was fixed within a month.
Attachment #118751 - Flags: superreview?(bzbarsky) → superreview+
Attachment #118699 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 118699 [details] [diff] [review] fix r=jag
Attachment #118699 - Flags: review?(jaggernaut) → review+
The v.02 patch has a Mat.round that should be Math.round.
Comment on attachment 118699 [details] [diff] [review] fix checked in
Attachment #118699 - Attachment is obsolete: true
This is a low risk patch that improves performance of the download dialog. I've used the patch that was just checked into the trunk for months without any problems or JS warnings, and it deserves to go into the 1.4 branch.
Comment on attachment 118699 [details] [diff] [review] fix moving approval request forward.
Attachment #118699 - Flags: approval1.4? → approval1.4.x?
This was checked in months ago.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
changing summary to reflect what file the patch really touched
Summary: parseInt() used instead of Math.round() or Math.floor() in xpfe/components/ucth/resources/helperAppDldProgress.js → parseInt() used instead of Math.round() or Math.floor() in embedding/components/ui/progressDlg/nsProgressDlg.js
You need to log in before you can comment on or make changes to this bug.