1414688 - JSON viewer should not be opened automatically when opening about:telemetry in RTL locales

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[magicp]

Steps to reproduce:
1. Start Firefox in RTL locales (e.g. ar)
2. Open "about:telemetry"

Actual Results:
JSON viewer will be opened automatically.

Expected Results:
JSON viewer should not be opened automatically in RTL locales.

Comment 1

[Aditya Bharti [:adbugger]]

Hi,
I would like to work on this bug. Can you tell how to start Firefox in RTL mode? For now, I am doing these two things:
1. Changing intl.uidirection = 1 in about:config
2. Going to about:telemetry and typing document.body.dir = 'rtl' in the console.

While everything is in RTL layout, I am unable to reproduce the error. I suspect it only happens when you change the locale before starting up? Could you tell me how to emulate that behaviour without changing the language, since I don't know any language that requires RTL?

Also, in general, how do I change the locale in Nightly? I worked on a similar bug recently and needed to do the same.

Comment 2

[Axel Hecht [:Pike]]

https://dxr.mozilla.org/mozilla-central/source/toolkit/content/aboutTelemetry.js#1154 is the code, and the error message is:

hgram.buckets is undefined

Tested on an Arabic mac Nightly.

Comment 3

[Aditya Bharti [:adbugger]]

Attachment: Histogram Object Structure

The hgram object has no property 'buckets'. hgram.values exists and hgram.values.reverse() does not throw any errors. For now, simply commenting out the 'hgram.buckets.reverse()' line works. 

I understand that the values in a histogram need to be reversed for RTL modes but I also think that the buckets is a remnant from a previous version of the code with a specific function. What is that line supposed to do? The buckets property is not mentioned anywhere else in aboutTelemetry.js.

Comment 4

[Aditya Bharti [:adbugger]]

Attachment: hgram.buckets not defined. Deleting line.

Alright, so according to :Dexter from #telemetry, this is indeed a remnant from older versions when histograms had a 'bucket' field. That was refactored in bug 112480: https://bugzilla.mozilla.org/show_bug.cgi?id=1122480.

The appropriate fix for this seems to be deleting the 'hgram.buckets.reverse()' line. Uploading initial patch. Please take a look.

Comment 5

[Chris H-C :chutten]

Do the histograms displayed by about:telemetry have the low-value buckets on the right with your change?

Comment 6

[Aditya Bharti [:adbugger]]

Attachment: Lower indexed buckets on the right

Yes, it looks like the lower valued buckets are on the right with my change. Screenshot attached.

Comment 7

[Chris H-C :chutten]

Excellent! Form this into a commit with a good message and I'll give it a review.

Comment 8

[Aditya Bharti [:adbugger]]

Attachment: Removed call to hgram.buckets.reverse().

Submitting patch for review.

Comment 9

[Chris H-C :chutten]

Comment on attachment 8929042 [details] [diff] [review]
Removed call to hgram.buckets.reverse().

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

Looks good to me!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8ea0fdcd066
Fix about:telemetry page in RTL locales opens JSON viewer. r=chutten

Comment 11

[Andrei Ciure[:aciure]]

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

Comment 12

[Mohammad Maruf Rahman]

I have reprodcued this bug with Nightly 58.0a1 (2017-11-05) -  AR on Windows 10, 6 Bit!

This bug's fix is verified with Latest Latest Nightly (Arabic build)!

Build ID  : 20171201100115
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0