| Summary: | Add default implementation for formatLocalizedString() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Component: | Platform | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Status: | ASSIGNED --- | ||||||||
| Severity: | Normal | CC: | Basuke.Suzuki, dbates, ews-watchlist, Hironori.Fujii, sam, thorton, timothy | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Basuke Suzuki
2018-07-12 14:48:11 PDT
Created attachment 344884 [details]
PATCH
Comment on attachment 344884 [details] PATCH Attachment 344884 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8522408 New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html Created attachment 344918 [details]
Archive of layout-test-results from ews201 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
There is a comment in the file. > // We can't use String::format for two reasons: > // 1) It doesn't handle non-ASCII characters in the format string. > // 2) It doesn't handle the %2$d syntax. > // Note that because |format| is used as the second parameter to va_start, it cannot be a reference > // type according to section 18.7/3 of the C++ N1905 standard. If you implement this function by using vsnprintf, there is no reason not to use String::format which is also using vsnprintf. Note: There is a unresolved bug in String::format. (Bug 186758) I have a question. What will happen if String::format is given non-ASCII charactors? (In reply to Fujii Hironori from comment #4) > There is a comment in the file. > > > // We can't use String::format for two reasons: > > // 1) It doesn't handle non-ASCII characters in the format string. > > // 2) It doesn't handle the %2$d syntax. > > // Note that because |format| is used as the second parameter to va_start, it cannot be a reference > > // type according to section 18.7/3 of the C++ N1905 standard. > > If you implement this function by using vsnprintf, there is no > reason not to use String::format which is also using vsnprintf. Oh, that's right. > I have a question. What will happen if String::format is given > non-ASCII charactors? AFAIK, format string is okay with utf-8. An issue may happen when using string parameter with length like '%3s' format directive. I bet it won't truncate 'あいうえお' into 'あいう' but just 'あ' ( because it encoded int \xe3, \x81, \x82). Bigger issue, though, may be the lack of supporting '%2$d' syntax, parameter field https://en.wikipedia.org/wiki/Printf_format_string#Parameter_field This is very important for localization. |