Closed
Bug 593046
Opened 14 years ago
Closed 14 years ago
nsJapaneseToUnicode uses the pref service off of the main thread
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: sdwilsh, Assigned: hsivonen)
References
Details
Attachments
(1 file, 1 obsolete file)
7.03 KB,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
REFTEST TEST-START | file:///home/cltbld/talos-slave/tryserver_fedora-debug_test-crashtest/build/reftest/tests/intl/uconv/crashtests/563618.html ++DOMWINDOW == 49 (0x9514d10) [serial = 869] [outer = 0x8bb91a0] ###!!! ASSERTION: Cannot be used on a background thread!: 'Error', file /builds/slave/tryserver-linux-debug/build/modules/libpref/src/nsPrefService.cpp, line 871 nsPrefService::CheckAndLogBackgroundThreadUse [modules/libpref/src/nsPrefService.cpp:873] nsPrefBranch::GetCharPref [modules/libpref/src/nsPrefBranch.cpp:219] nsPrefService::GetCharPref [modules/libpref/src/nsPrefService.h:60] nsJapaneseToUnicode::setMapMode [intl/uconv/ucvja/nsJapaneseToUnicode.cpp:64] nsEUCJPToUnicodeV2::nsEUCJPToUnicodeV2 [intl/uconv/ucvja/nsJapaneseToUnicode.h:96] nsEUCJPToUnicodeV2Constructor [intl/uconv/src/nsUConvModule.cpp:437] mozilla::GenericFactory::CreateInstance [GenericFactory.cpp:49] nsComponentManagerImpl::CreateInstanceByContractID [xpcom/components/nsComponentManager.cpp:1284] CallCreateInstance [nsComponentManagerUtils.cpp:170] nsCreateInstanceByContractID::operator() [nsComponentManagerUtils.cpp:210] nsCOMPtr<nsIUnicodeDecoder>::assign_from_helper [nsCOMPtr.h:1272] nsCOMPtr<nsIUnicodeDecoder>::operator= [nsCOMPtr.h:731] nsCharsetConverterManager::GetUnicodeDecoderRaw [intl/uconv/src/nsCharsetConverterManager.cpp:233] nsHtml5MetaScanner::tryCharset [parser/html/nsHtml5MetaScannerCppSupplement.h:125] nsHtml5MetaScanner::tryCharset [parser/html/nsHtml5MetaScanner.cpp:699] nsHtml5MetaScanner::stateLoop [parser/html/nsHtml5MetaScanner.cpp:327] nsHtml5MetaScanner::sniff [parser/html/nsHtml5MetaScannerCppSupplement.h:69] nsHtml5StreamParser::SniffStreamBytes [parser/html/nsHtml5StreamParser.cpp:437] nsHtml5StreamParser::DoDataAvailable [parser/html/nsHtml5StreamParser.cpp:682] nsHtml5DataAvailable::Run [parser/html/nsHtml5StreamParser.cpp:721] nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547] NS_ProcessNextEvent_P [nsThreadUtils.cpp:250] nsThread::ThreadFunc [xpcom/threads/nsThread.cpp:262] _pt_root [nsprpub/pr/src/pthreads/ptthread.c:231] libpthread.so.0 + 0x5ab5 Also happens in: REFTEST TEST-START | file:///home/cltbld/talos-slave/tryserver_fedora-debug_test-crashtest/build/reftest/tests/gfx/tests/crashtests/122875-1.html ###!!! ASSERTION: Cannot be used on a background thread!: 'Error', file /builds/slave/tryserver-linux-debug/build/modules/libpref/src/nsPrefService.cpp, line 871 nsPrefService::CheckAndLogBackgroundThreadUse [modules/libpref/src/nsPrefService.cpp:873] nsPrefBranch::GetCharPref [modules/libpref/src/nsPrefBranch.cpp:219] nsPrefService::GetCharPref [modules/libpref/src/nsPrefService.h:60] nsJapaneseToUnicode::setMapMode [intl/uconv/ucvja/nsJapaneseToUnicode.cpp:64] nsShiftJISToUnicode::nsShiftJISToUnicode [intl/uconv/ucvja/nsJapaneseToUnicode.h:61] nsShiftJISToUnicodeConstructor [intl/uconv/src/nsUConvModule.cpp:436] mozilla::GenericFactory::CreateInstance [GenericFactory.cpp:49] nsComponentManagerImpl::CreateInstanceByContractID [xpcom/components/nsComponentManager.cpp:1284] CallCreateInstance [nsComponentManagerUtils.cpp:170] nsCreateInstanceByContractID::operator() [nsComponentManagerUtils.cpp:210] nsCOMPtr<nsIUnicodeDecoder>::assign_from_helper [nsCOMPtr.h:1272] nsCOMPtr<nsIUnicodeDecoder>::operator= [nsCOMPtr.h:731] nsCharsetConverterManager::GetUnicodeDecoderRaw [intl/uconv/src/nsCharsetConverterManager.cpp:233] nsHtml5MetaScanner::tryCharset [parser/html/nsHtml5MetaScannerCppSupplement.h:125] nsHtml5MetaScanner::tryCharset [parser/html/nsHtml5MetaScanner.cpp:699] nsHtml5MetaScanner::stateLoop [parser/html/nsHtml5MetaScanner.cpp:327] nsHtml5MetaScanner::sniff [parser/html/nsHtml5MetaScannerCppSupplement.h:69] nsHtml5StreamParser::SniffStreamBytes [parser/html/nsHtml5StreamParser.cpp:451] nsHtml5StreamParser::DoDataAvailable [parser/html/nsHtml5StreamParser.cpp:682] nsHtml5DataAvailable::Run [parser/html/nsHtml5StreamParser.cpp:721] nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547] NS_ProcessNextEvent_P [nsThreadUtils.cpp:250] nsThread::ThreadFunc [xpcom/threads/nsThread.cpp:262] _pt_root [nsprpub/pr/src/pthreads/ptthread.c:231] libpthread.so.0 + 0x5ab5
Reporter | ||
Comment 1•14 years ago
|
||
Given the stacks, I suspect this is a regression from the HTML5 parser doing things off of the main thread.
Blocks: 482918
blocking2.0: --- → final+
Reporter | ||
Comment 2•14 years ago
|
||
Also happens in: REFTEST TEST-START | file:///Users/cltbld/talos-slave/tryserver_snowleopard-debug_test-reftest/build/reftest/tests/layout/reftests/bugs/109735-1.html REFTEST TEST-START | file:///Users/cltbld/talos-slave/tryserver_snowleopard-debug_test-reftest/build/reftest/tests/layout/reftests/bugs/109735-1-ref.html REFTEST TEST-START | file:///Users/cltbld/talos-slave/tryserver_snowleopard-debug_test-reftest/build/reftest/tests/layout/reftests/bugs/116882-1.html REFTEST TEST-START | file:///Users/cltbld/talos-slave/tryserver_snowleopard-debug_test-reftest/build/reftest/tests/layout/reftests/bugs/116882-1-ref.html
Comment 3•14 years ago
|
||
Can we just remove the pref? From what I can tell, this pref is set to one value on Windows, a different value on OS/2, and not set on other platforms. It seems really strange to me that we need platform-specific decoding which might affect the web, but if we really do, let's just hardcode it an skip having a pref altogether?
Comment 4•14 years ago
|
||
(In reply to comment #3) > Can we just remove the pref? See discussion in bug 108136
Comment 5•14 years ago
|
||
I really don't think that's a convincing argument, can we remove the pref?
Comment 6•14 years ago
|
||
(In reply to comment #5) > I really don't think that's a convincing argument, can we remove the pref? Which part isn't convincing, having the platform difference in the first place or using a pref instead of platform ifdefs? We need input from Japanese experts here.
Comment 7•14 years ago
|
||
It's 2010. If we still need the platform ifdefs because of bad default fonts, that sucks but whatever. It's the pref that should go.
Reporter | ||
Comment 8•14 years ago
|
||
(In reply to comment #6) > Which part isn't convincing, having the platform difference in the first place > or using a pref instead of platform ifdefs? IMO, using a pref instead of platform ifdefs.
Comment 9•14 years ago
|
||
The first reason of bug 108136 comment 33 which is for Unix users makes sense for me. Any fonts are not bad because each font is created for a platform. However, I'm not sure whether that is still problem for Unix users too. All of Linux distributions are using UTF-8 for their default encoding now. And even if that is still problem, such troubles should happen on other applications too. I don't think that it should be fixed by prefs only on Gecko.
Assignee | ||
Comment 10•14 years ago
|
||
Making the pref go away is the most important thing, but it's totally bogus for Shift_JIS in Web content to decode differently in a DOM-exposed way depending on the platform the browser runs on.
Assignee | ||
Comment 11•14 years ago
|
||
Updated•14 years ago
|
Attachment #471822 -
Attachment is patch: true
Attachment #471822 -
Attachment mime type: application/octet-stream → text/plain
Comment 12•14 years ago
|
||
Comment on attachment 471822 [details] [diff] [review] Untested fix > + // HTML5 says to use Windows-31J instead of the real Shift_JIS for decoding > + // XXX do we really want this for ISO-2022-JP and EUC-JP, too? Absolutely required. CP51932 and CP50220 are going to be registered for this purpose.
Assignee | ||
Comment 13•14 years ago
|
||
(In reply to comment #12) > Comment on attachment 471822 [details] [diff] [review] > Untested fix > > > + // HTML5 says to use Windows-31J instead of the real Shift_JIS for decoding > > + // XXX do we really want this for ISO-2022-JP and EUC-JP, too? > Absolutely required. CP51932 and CP50220 are going to be registered for this > purpose. OK. I removed the XXX comment. I didn't modify japanese.map since it is generated code. I'm not sure if part of it is now dead code.
Assignee: smontagu → hsivonen
Attachment #471822 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #473481 -
Flags: review?
Assignee | ||
Updated•14 years ago
|
Attachment #473481 -
Flags: review? → review?(smontagu)
Comment 14•14 years ago
|
||
Comment on attachment 473481 [details] [diff] [review] Fix using #ifdef instead of a pref So this is going to change behaviour on platforms other than OS/2 and Windows, correct? I tested (on Mac) that round-tripping still works, and I guess that is the important thing.
Attachment #473481 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 15•14 years ago
|
||
(In reply to comment #14) > So this is going to change behaviour on platforms other than OS/2 and Windows, > correct? I'm not sure, but having other platforms differ from Windows in how they decode bytes to UTF-16 would be bogus, so if Mac and Linux weren't doing the right thing already, the change should be good. Landed: http://hg.mozilla.org/mozilla-central/rev/570dc03cbf24
Assignee | ||
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•