Closed Bug 861487 Opened 7 years ago Closed 7 years ago

Sizes are displayed with non-localized decimal separator

Categories

(DevTools :: Netmonitor, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: u60234, Unassigned)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(2 files)

The network monitor show file and header sizes with a dot as decimal separator. This is wrong in many locales.
Is there something like a intl.decimal (similar to intl.ellipsis and friends) that I can use instead of creating a new property string in netmonitor.properties?
Use x.toLocaleString() where x is your floating point number.
(In reply to Dão Gottwald [:dao] from comment #2)

Thank you!
Regardless of my computer's region/format settings or firefox's language pack, toLocaleString seems to ignore them. No example on https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Number/toLocaleString worked.
'locales' seems to work (using 20.01)
(In reply to Stefan [:stefanh] from comment #5)
> Created attachment 737399 [details]
> Screenshot from error console
> 
> 'locales' seems to work (using 20.01)

That's curious.. Does it also work if you omit a specifying a locale?
(In reply to Victor Porof [:vp] from comment #6)
> (In reply to Stefan [:stefanh] from comment #5)
> > Created attachment 737399 [details]
> > Screenshot from error console
> > 
> > 'locales' seems to work (using 20.01)
> 
> That's curious.. Does it also work if you omit a specifying a locale?

I get the same result when evaluating 'var number = 123456.789; alert(number.toLocaleString());'
(with the sv-SE build of course)
(In reply to Victor Porof [:vp] from comment #4)
> Regardless of my computer's region/format settings or firefox's language
> pack, toLocaleString seems to ignore them.

Is this on Mac OS X? Number.toLocaleString has failed before there. See bug 368838.
(In reply to Hasse from comment #9)
> 
> Is this on Mac OS X? Number.toLocaleString has failed before there. See bug
> 368838.

Yes, OS X, 10.8.3.
(In reply to Victor Porof [:vp] from comment #10)
> (In reply to Hasse from comment #9)
> > 
> > Is this on Mac OS X? Number.toLocaleString has failed before there. See bug
> > 368838.
> 
> Yes, OS X, 10.8.3.

Ah, that explains it - I tried it on Win7 (attachment #737399 [details]). I just tried with 20.0 on Mac OS 10.7.5 and it doesn't work.
Bug 368838 hasn't seen any activity in almost 4 and a half years :( Any other suggestions besides toLocaleString?
You should really just use toLocaleString like we do in other places. Any solution to the bug on OS X shouldn't be specific to this bug.
(In reply to Dão Gottwald [:dao] from comment #13)
> You should really just use toLocaleString like we do in other places. Any
> solution to the bug on OS X shouldn't be specific to this bug.

That would mean this bug being fixed only for non OS X platforms, but ok.
Attached patch v1Splinter Review
Gross!
Attachment #737732 - Flags: review?(rcampbell)
Priority: -- → P3
Comment on attachment 737732 [details] [diff] [review]
v1

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

::: browser/devtools/netmonitor/netmonitor-view.js
@@ +882,5 @@
>     */
>    _addHeaders: function NVND__addHeaders(aName, aResponse) {
> +    let kb = aResponse.headersSize / 1024;
> +    let size = L10N.numberWithDecimals(kb, HEADERS_SIZE_DECIMALS);
> +    let text = L10N.getFormatStr("networkMenu.sizeKB", size);

ok. You could use a helper function to mush these functions together, but I wouldn't require it.

::: browser/devtools/netmonitor/test/browser_net_content-type.js
@@ +65,5 @@
>            status: 200,
>            statusText: "OK",
>            type: "png",
>            fullMimeType: "image/png",
> +          size: L10N.getFormatStr("networkMenu.sizeKB", 0.75),

/me squints

::: browser/devtools/shared/widgets/ViewHelpers.jsm
@@ +221,5 @@
> +    }
> +    // Remove {n} trailing decimals. Can't use toFixed(n) because
> +    // toLocaleString converts the number to a string. Also can't use
> +    // toLocaleString(, { maximumFractionDigits: n }) because it's not
> +    // implemented on OS X (bug 368838). Gross.

indeed!
Attachment #737732 - Flags: review?(rcampbell) → review+
(In reply to Rob Campbell [:rc] (:robcee) from comment #16)
> @@ +65,5 @@
> >            status: 200,
> >            statusText: "OK",
> >            type: "png",
> >            fullMimeType: "image/png",
> > +          size: L10N.getFormatStr("networkMenu.sizeKB", 0.75),
> 
> /me squints
> 

The previous implementation used toFixed, which does some lovely decimal rounding. The new implementation simply trims out decimals (but is localized, so yay >_>).
https://hg.mozilla.org/mozilla-central/rev/e6a7681887f7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.