861487 - Sizes are displayed with non-localized decimal separator

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[u60234]

The network monitor show file and header sizes with a dot as decimal separator. This is wrong in many locales.

Comment 1

[Victor Porof [:vporof][:vp]]

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?

Comment 2

[Dão Gottwald [::dao]]

Use x.toLocaleString() where x is your floating point number.

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Dão Gottwald [:dao] from comment #2)

Thank you!

Comment 4

[Victor Porof [:vporof][:vp]]

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.

Comment 5

[Stefan [:stefanh]]

Attachment: Screenshot from error console

'locales' seems to work (using 20.01)

Comment 6

[Victor Porof [:vporof][:vp]]

(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?

Comment 7

[Stefan [:stefanh]]

(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());'

Comment 8

[Stefan [:stefanh]]

(with the sv-SE build of course)

Comment 9

[u60234]

(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.

Comment 10

[Victor Porof [:vporof][:vp]]

(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.

Comment 11

[Stefan [:stefanh]]

(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.

Comment 12

[Victor Porof [:vporof][:vp]]

Bug 368838 hasn't seen any activity in almost 4 and a half years :( Any other suggestions besides toLocaleString?

Comment 13

[Dão Gottwald [::dao]]

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.

Comment 14

[Victor Porof [:vporof][:vp]]

(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.

Comment 15

[Victor Porof [:vporof][:vp]]

Attachment: v1

Gross!

Comment 16

[Rob Campbell [:rc] (:robcee)]

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!

Comment 17

[Victor Porof [:vporof][:vp]]

(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 >_>).

Comment 18

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/e6a7681887f7

Comment 19

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/e6a7681887f7