Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

toLocale* methods not respecting locale #5879

Closed
alexwykoff opened this issue Nov 28, 2016 · 10 comments
Closed

toLocale* methods not respecting locale #5879

alexwykoff opened this issue Nov 28, 2016 · 10 comments

Comments

@alexwykoff
Copy link
Contributor

alexwykoff commented Nov 28, 2016

Did you search for similar issues before submitting this one?
No

Describe the issue you encountered:
When trying the 0.13.0 Preview1 build, I found the new tab page showed a lot more info than before.
Stats are wrapping.

Expected behavior:
New tab page should look the same as before.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Tested on OS X

  • Brave Version:
    0.13.0 Preview 1

  • Steps to reproduce:

    1. Open a new tab.
  • Screenshot if needed:

screen shot 2016-11-28 at 1 49 56 pm

http://jsbin.com/tihayoy/edit?html,js,output

@bsclifton
Copy link
Member

bsclifton commented Nov 28, 2016

The page is using Date.prototype.toLocaleTimeString() which may be causing the issue

This would be a good chance to also use moment.js, in a similar fashion to what's implemented in the ledger (Brave Payments) code

@bsclifton
Copy link
Member

bsclifton commented Nov 29, 2016

After talking this out more, it could be a bigger problem. The formatting here should be picked up properly. Current production builds use the same locale that is picked in about:preferences and functions like Number.prototype.toLocaleString() and Date.prototype.toLocaleTimeString() work as expected

We may need to enable internationalization when building node/v8:
https://chromium.googlesource.com/external/github.com/v8/node/+/v4.2.5#ECMA_402_support

cc: @bridiver @posix4e

@bsclifton
Copy link
Member

bsclifton commented Nov 30, 2016

Here's a JSBin you can execute to show that no other locales are working
http://jsbin.com/tihayoy/edit?html,js,output

in the 54 build, it returns 10:15:27 GMT-0800 (Pacific Standard Time)
in 53, it properly returns 上午11:16:07

@bsclifton bsclifton changed the title New tab page showing seconds and GMT offset, stats are wrapping toLocale* methods not respecting locale Dec 2, 2016
@bsclifton
Copy link
Member

also affects history and likely affects bookmarks manager; see:
#5932

image

@bsclifton bsclifton assigned bsclifton and unassigned bsclifton Dec 10, 2016
@bsclifton
Copy link
Member

OK this one has been not so fun. I want to take a few minutes and document the problem and what I've tried to solve it.

building Muon (our fork of Electron) from source

Building browser-laptop-bootstrap (not public yet) will do 2 major steps:

  • compile node using node-gyp
  • compile muon using gn

The build has a structure like this:

browser-laptop-bootstrap
- src
  - electron (brave/electron)
    - vendor
      - node (brave/node#chromium54)
  - icu (from Chromium)
  - v8 (from Chromium)

Both Node and Muon depend on v8 and must use the same version (if you try to include both, there'll be an issue since singleton classes are involved). The previous Electron (before gn builds) was compiling node without i18n support because it uses the resource files from libchromiumcontent (which no longer exists for us).

The solution to all of this is: update our fork of Node to use the same v8 that's included in Chromium.

We need to build v8 with international support. Here are some diffs from what I think should solve the issue:

brave/browser-laptop-bootstrap

diff --git a/lib/config.js b/lib/config.js
index 1644b12..c5817eb 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -48,7 +48,8 @@ Config.prototype.buildArgs = function () {
     clang_use_chrome_plugins: false,
     enable_print_preview: false,
     enable_plugin_installation: false,
-    // enable_stripping: false,
+    enable_stripping: false,
+    icu_use_data_file: false,
     enable_supervised_users: false,
     branding_path_component: "brave",
     enable_widevine: process.platform !== 'linux',

brave/electron

diff --git a/BUILD.gn b/BUILD.gn
index 80d330b..9add40c 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -368,14 +368,13 @@ if (enable_hidpi) {
   ]
 }
 
-if (icu_use_data_file) {
-  electron_framework_sources += [
-    "$root_out_dir/icudtl.dat"
-  ]
-  electron_framework_public_deps += [
-    "//third_party/icu:icudata"
-  ]
-}
+# icu is already built (when node was built with gyp)
+electron_framework_sources += [
+  "$root_out_dir/icudtl.dat"
+]
+electron_framework_public_deps += [
+  "//third_party/icu:icudata"
+]
 
 if (v8_use_external_startup_data) {
   electron_framework_sources += [
diff --git a/build/node/node.gypi b/build/node/node.gypi
index 4c88b93..8017037 100644
--- a/build/node/node.gypi
+++ b/build/node/node.gypi
@@ -32,7 +32,9 @@
     'uv_use_dtrace': 'false',
     'V8_BASE': '',
     'v8_postmortem_support': 'false',
-    'v8_enable_i18n_support': 0,
+    'v8_enable_i18n_support': 1,
+    'icu_small': 'false',
+    'icu_use_data_file_flag': 1,
     'v8_inspector': 'false',
     'library': 'static_library',
     'msvs_use_common_release': 0,
diff --git a/build/node/v8.gypi b/build/node/v8.gypi
index 3b28244..ed97c62 100644
--- a/build/node/v8.gypi
+++ b/build/node/v8.gypi
@@ -11,6 +11,7 @@
    'icu_libraries': '["icui18n", "icuuc"]',
    'v8_use_snapshot': 'true',
    'v8_use_external_startup_data': 1,
+   'icu_use_data_file_flag': 1
  },
  'target_defaults': {
    'msvs_disabled_warnings': [

Current status

On macOS, this compiles and links but crashes with this error being written to the console:
[1218/165024:ERROR:mach_port_broker.mm(43)] bootstrap_look_up: Unknown service name (1102)

macOS will bring up a dialog to report the error, from which you can get a crash report. Here are the relevant bits:

Process:               Brave [51481]
Path:                  /Users/USER/Documents/*/Brave.app/Contents/MacOS/Brave
Identifier:            org.brave.Brave
Version:               2.0.4.0 (4.0)
Code Type:             X86-64 (Native)
Parent Process:        node [51480]
Responsible:           Brave [51481]
User ID:               501

Date/Time:             2016-12-18 16:50:55.546 -0700
OS Version:            Mac OS X 10.12.1 (16B2555)
Report Version:        12
Anonymous UUID:        62D5217E-0F4F-AE4A-9F7F-1C22D966002F

Sleep/Wake UUID:       E3B74642-D24B-4ADF-88B4-28306A294FF4

Time Awake Since Boot: 410000 seconds
Time Since Wake:       63000 seconds

System Integrity Protection: enabled

Crashed Thread:        0  CrBrowserMain  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x000000000000029f
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [0]

VM Regions Near 0x29f:
--> 
    __TEXT                 0000000108a6e000-0000000108a6f000 [    4K] r-x/rwx SM=COW  /Users/USER/Documents/*/Brave.app/Contents/MacOS/Brave

Application Specific Information:
dyld: in dladdr()

Thread 0 Crashed:: CrBrowserMain  Dispatch queue: com.apple.main-thread
0   libnode.dylib                 	0x00000001122b326a v8::internal::ExternalOneByteStringUtf16CharacterStream::FillBuffer(unsigned long) + 106 (scanner-character-streams.cc:506)
1   libnode.dylib                 	0x00000001122b1a3a v8::internal::BufferedUtf16CharacterStream::ReadBlock() + 58 (scanner-character-streams.cc:141)
2   libnode.dylib                 	0x00000001122b4974 void v8::internal::Scanner::Advance<false, true>() + 36 (scanner.h:46)
3   libnode.dylib                 	0x00000001122b3915 v8::internal::Scanner::Initialize(v8::internal::Utf16CharacterStream*) + 21 (scanner.h:518)
4   libnode.dylib                 	0x00000001122633ba v8::internal::Parser::DoParseLazy(v8::internal::ParseInfo*, v8::internal::AstRawString const*, v8::internal::Utf16CharacterStream*) + 42 (parser-base.h:699)
5   libnode.dylib                 	0x0000000112263192 v8::internal::Parser::ParseLazy(v8::internal::Isolate*, v8::internal::ParseInfo*) + 946 (parser.cc:968)
6   libnode.dylib                 	0x000000011227e8b0 v8::internal::Parser::Parse(v8::internal::ParseInfo*) + 144 (parser.cc:5257)
7   libnode.dylib                 	0x000000011227e79e v8::internal::Parser::ParseStatic(v8::internal::ParseInfo*) + 46 (parser.cc:5232)
8   libnode.dylib                 	0x0000000111fb139d v8::internal::(anonymous namespace)::GetUnoptimizedCode(v8::internal::CompilationInfo*) + 125 (compiler.cc:547)
9   libnode.dylib                 	0x0000000111faf552 v8::internal::Compiler::Compile(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ClearExceptionFlag) + 1186 (handles.h:220)
10  libnode.dylib                 	0x000000011231e8df v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*) + 255 (runtime-compiler.cc:37)
11  ???                           	0x00000e3529a063a7 0 + 15621494432679
12  ???                           	0x00000e3529a0675a 0 + 15621494433626
13  ???                           	0x00000e3529a76a08 0 + 15621494893064
14  ???                           	0x00000e3529a4a7c3 0 + 15621494712259
15  ???                           	0x00000e3529a2aa81 0 + 15621494581889
16  libnode.dylib                 	0x00000001120c0ef4 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::Object>) + 820 (execution.cc:141)
17  libnode.dylib                 	0x00000001120c0ba9 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) + 169 (execution.cc:178)
18  libnode.dylib                 	0x0000000111da2252 v8::internal::Genesis::CallUtilsFunction(v8::internal::Isolate*, char const*) + 354 (bootstrapper.cc:2294)
19  libnode.dylib                 	0x0000000111da7008 v8::internal::Genesis::InstallExperimentalNatives() + 424 (bootstrapper.cc:3389)
20  libnode.dylib                 	0x0000000111da8f9b v8::internal::Genesis::Genesis(v8::internal::Isolate*, v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>, v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*, unsigned long, v8::internal::GlobalContextType) + 1355 (bootstrapper.cc:3988)
21  libnode.dylib                 	0x0000000111d96e49 v8::internal::Bootstrapper::CreateEnvironment(v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>, v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*, unsigned long, v8::internal::GlobalContextType) + 89 (bootstrapper.cc:153)
22  libnode.dylib                 	0x0000000111d456c8 v8::NewContext(v8::Isolate*, v8::ExtensionConfiguration*, v8::MaybeLocal<v8::ObjectTemplate>, v8::MaybeLocal<v8::Value>, unsigned long) + 600 (api.cc:5617)
23  org.brave.Brave.framework     	0x00000001094fc218 atom::JavascriptEnvironment::JavascriptEnvironment() + 88 (v8.h:7822)
24  org.brave.Brave.framework     	0x00000001094f1620 atom::AtomBrowserMainParts::PreMainMessageLoopRun() + 64 (memory:2771)
25  org.brave.Brave.framework     	0x0000000108c9bc73 content::BrowserMainLoop::PreMainMessageLoopRun() + 67 (trace_event.h:1016)
26  org.brave.Brave.framework     	0x0000000108f86050 content::StartupTaskRunner::RunAllTasksNow() + 48 (callback.h:388)
27  org.brave.Brave.framework     	0x0000000108c9a403 content::BrowserMainLoop::CreateStartupTasks() + 579 (trace_event.h:1016)
28  org.brave.Brave.framework     	0x0000000108c9e49a content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) + 538 (memory:2752)
29  org.brave.Brave.framework     	0x0000000108c97c64 content::Browse

@bridiver
Copy link
Collaborator

fixed, just narrowing down what was actually needed

@bbondy
Copy link
Member

bbondy commented Dec 22, 2016

Re-opening for Windows.

@posix4e
Copy link
Contributor

posix4e commented Dec 22, 2016

Can confirm linux is a problem
screenshot from 2016-12-22 11-55-20

@luixxiul
Copy link
Contributor

Test plan: available on the 1st post.

@luixxiul
Copy link
Contributor

I added release-notes/exclude as the issue appeared and was fixed within the same milestone.

petemill pushed a commit to brave/brave-core that referenced this issue Jul 27, 2020
petemill pushed a commit to brave/brave-core that referenced this issue Jul 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants