Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wxColour's string CTOR and GetAsString(wxC2S_CSS_SYNTAX) determine incorrect alpha value with German locale #13077

Closed
wxtrac opened this issue Mar 23, 2011 · 3 comments
Labels
base Affecting non-GUI code v3.0

Comments

@wxtrac
Copy link
Collaborator

wxtrac commented Mar 23, 2011

Issue migrated from trac ticket # 13077

component: base | priority: normal | resolution: fixed

2011-03-23 20:16:14: @Blake-Madden created the issue


If you pass in "rgba(100,100,100, 0.4)" into a wxColour under German regional settings, the alpha value will be set to 0, not 100. Likewise, calling GetAsString(wxC2S_CSS_SYNTAX) will return a bogus alpha value in the returned string.

Attached patch fixes this. In the CTOR, instead of calling Sscanf, the string is tokenized and each number string is safely converted using ToCLong and ToCDouble. For GetAsSting, FromCDouble is now called instead of using a "%.3f" to printf.

Tested under German and English locales, results are the same under both now.

@wxtrac
Copy link
Collaborator Author

wxtrac commented Mar 23, 2011

2011-03-23 20:16:29: @Blake-Madden uploaded file update.diff (2.1 KiB)

Fix patch

@wxtrac
Copy link
Collaborator Author

wxtrac commented Mar 31, 2011

2011-03-31 10:23:49: @vadz changed status from new to confirmed

2011-03-31 10:23:49: @vadz commented

Unfortunately using wxStringTokenize() here correctly is difficult as you either need to use wxTOKEN_RET_EMPTY (which your version did, by default) and then you refuse strings with multiple consecutive whitespaces (which are allowed by the spec) or you use wxTOKEN_STRTOK and then you allow them but also allow string with multiple consecutive commas which is clearly wrong. So I used a sscanf()-based solution after all, even if it is pretty ugly as well due to the fact that there may or not be a whitespace between the last number and the closing parenthesis. But I don't see any better way to do it and at least this passes the tests (BTW, your original version didn't pass the test for parsing RGB as you changed the types of the values to long but didn't replace %d with %ld in sscanf() call -- hence the usefulness of having tests even for something as trivial as this as I definitely wouldn't have caught it myself).

Anyhow, I'll check in my fix soon.

@wxtrac
Copy link
Collaborator Author

wxtrac commented Mar 31, 2011

2011-03-31 11:28:42: @vadz changed status from confirmed to closed

2011-03-31 11:28:42: @vadz set resolution to fixed

2011-03-31 11:28:42: @vadz commented

(In [67356]) Fix alpha handling in CSS syntax in wxColour in non-"C" locale.

Use locale-independent functions to parse and generate the floating point
alpha representation in CSS syntax for colours to make it work in locales
which don't use period as decimal separator.

Closes #13077.

@wxtrac wxtrac closed this as completed Mar 31, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base Affecting non-GUI code v3.0
Projects
None yet
Development

No branches or pull requests

1 participant