This bug is to track other changes made or yet to be made in order to enable 8 bit strings in JavaScriptCore. There are several small changes that need to be made to "turn on" the 8 bit path in code that has been or will be checked in.
There are several "FIXME" occurrences in Source/JavaScriptCore/parser/Lexer.cpp that need to be changed as part of turning on 8 bit strings. These are in the methods append8(), parseIdentifier() and parseString().
In Lexer.cpp, parseIdentifier() was refactored and now only the #if 0 needs to be removed.
Created attachment 115402 [details] Patch for review This patch may have export issues building on windows.
Comment on attachment 115402 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=115402&action=review r=me > Source/JavaScriptCore/ChangeLog:13 > + type. This change riplled into WebCore code as well. Typo: riplled. > Source/JavaScriptCore/runtime/Identifier.cpp:172 > + if (string.is8Bit()) { Would be nice to have a template helper for this, to achieve only one copy of the code vs two. > Source/JavaScriptCore/wtf/text/StringImpl.cpp:540 > + const LChar* from = characters8(); This is another candidate for a template helper function. > Source/JavaScriptCore/wtf/text/StringImpl.cpp:-1351 > - if (!bc) This is another candidate for a template helper function -- would have only needed to fix this bug in one place instead of two. > Source/JavaScriptCore/wtf/text/StringImpl.cpp:1579 > +PassRefPtr<StringImpl> StringImpl::adopt(StringBuffer<LChar>& buffer) This is another candidate for templatization.
(In reply to comment #4) > (From update of attachment 115402 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=115402&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:13 > > + type. This change riplled into WebCore code as well. > > Typo: riplled. Fixed. > > Source/JavaScriptCore/runtime/Identifier.cpp:172 > > + if (string.is8Bit()) { > > Would be nice to have a template helper for this, to achieve only one copy of the code vs two. Done. > > Source/JavaScriptCore/wtf/text/StringImpl.cpp:540 > > + const LChar* from = characters8(); > > This is another candidate for a template helper function. Done. > > Source/JavaScriptCore/wtf/text/StringImpl.cpp:-1351 > > - if (!bc) > > This is another candidate for a template helper function -- would have only needed to fix this bug in one place instead of two. Done. > > Source/JavaScriptCore/wtf/text/StringImpl.cpp:1579 > > +PassRefPtr<StringImpl> StringImpl::adopt(StringBuffer<LChar>& buffer) > > This is another candidate for templatization. Didn't template this case due to making StringImpl.h dependent on StringBuffer.h. Tried putting only declaration in StringImpl.h and the implementation in StringImpl.cpp and then explicitly instatiate the two flavors, but that creates symbols with Weak references that doesn't compile. Given these various issues, felt that the implementation in the original patch was preferred.
Committed r100510: <http://trac.webkit.org/changeset/100510>
(In reply to comment #6) > Committed r100510: <http://trac.webkit.org/changeset/100510> I don't know how, but it broke 4 tests on the Qt bot: --- /ramdisk/qt-linux-64-release/build/layout-test-results/editing/selection/find-yensign-and-backslash-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/editing/selection/find-yensign-and-backslash-actual.txt @@ -2,12 +2,12 @@ Results -We can find a backslash in EUC-JP page by finding a yen sign: PASS -We can find a backslash in EUC-JP text control by finding a yen sign: PASS -We can find a backslash in Shift_JIS page by finding a yen sign: PASS -We can find a backslash in Shift_JIS text control by finding a yen sign: PASS -We can find a backslash in ISO-2022-JP page by finding a yen sign: PASS -We can find a backslash in ISO-2022-JP text control by finding a yen sign: PASS +We can find a backslash in EUC-JP page by finding a yen sign: FAIL +We can find a backslash in EUC-JP text control by finding a yen sign: FAIL +We can find a backslash in Shift_JIS page by finding a yen sign: FAIL +We can find a backslash in Shift_JIS text control by finding a yen sign: FAIL +We can find a backslash in ISO-2022-JP page by finding a yen sign: FAIL +We can find a backslash in ISO-2022-JP text control by finding a yen sign: FAIL We can NOT find a backslash in UTF8 page by finding a yen sign: PASS We can NOT find a backslash in UTF8 text control by finding a yen sign: PASS --- /ramdisk/qt-linux-64-release/build/layout-test-results/editing/selection/find-yensign-and-backslash-with-japanese-fonts-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/editing/selection/find-yensign-and-backslash-with-japanese-fonts-actual.txt @@ -1,24 +1,24 @@ No specified font + yen sign: PASS No specified font + backslash: PASS -MS PGothic + yen sign: PASS +MS PGothic + yen sign: FAIL MS PGothic + backslash: PASS -MS Gothic + yen sign: PASS +MS Gothic + yen sign: FAIL MS Gothic + backslash: PASS -MS PMincho + yen sign: PASS +MS PMincho + yen sign: FAIL MS PMincho + backslash: PASS -MS Mincho + yen sign: PASS +MS Mincho + yen sign: FAIL MS Mincho + backslash: PASS -Meiryo + yen sign: PASS +Meiryo + yen sign: FAIL Meiryo + backslash: PASS -ďźďźł ďź°ă�´ă�ˇă��ă�Ż + yen sign: PASS +ďźďźł ďź°ă�´ă�ˇă��ă�Ż + yen sign: FAIL ďźďźł ďź°ă�´ă�ˇă��ă�Ż + backslash: PASS -ďźďźł ă�´ă�ˇă��ă�Ż + yen sign: PASS +ďźďźł ă�´ă�ˇă��ă�Ż + yen sign: FAIL ďźďźł ă�´ă�ˇă��ă�Ż + backslash: PASS -ďźďźł ďź°ć��ć�� + yen sign: PASS +ďźďźł ďź°ć��ć�� + yen sign: FAIL ďźďźł ďź°ć��ć�� + backslash: PASS -ďźďźł ć��ć�� + yen sign: PASS +ďźďźł ć��ć�� + yen sign: FAIL ďźďźł ć��ć�� + backslash: PASS -ă�Ąă�¤ă�Şă�Ş + yen sign: PASS +ă�Ąă�¤ă�Şă�Ş + yen sign: FAIL ă�Ąă�¤ă�Şă�Ş + backslash: PASS Times + yen sign: PASS Times + backslash: PASS --- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/text/find-kana-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/text/find-kana-actual.txt @@ -27,7 +27,7 @@ PASS canFind(katakanaLetterGa, katakanaLetterGa) is true PASS canFind(katakanaLetterKa, katakanaLetterKa) is true PASS canFind(katakanaLetterSmallA, katakanaLetterSmallA) is true -PASS canFind(latinCapitalLetterAWithGrave, latinCapitalLetterAWithGrave) is true +FAIL canFind(latinCapitalLetterAWithGrave, latinCapitalLetterAWithGrave) should be true. Was false. Hiragana, katakana, and half width katakana: Must be treated as equal --- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/text/find-spaces-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/text/find-spaces-actual.txt @@ -1,1 +1,1 @@ -SUCCESS: Found all the spaces as expected. +FAILURE: Cannot find space. Cannot find two spaces in a <pre> element. Cannot find an when searching for a space. Cannot find a space followed by when searching for two spaces. Cannot find an followed by a space when searching for two spaces.
> > Committed r100510: <http://trac.webkit.org/changeset/100510> > > I don't know how, but it broke 4 tests on the Qt bot: The problem was fixed in bug 72602.