1516101 - ProfileManager: Theme parsing error due to using locale-specific function AppendPrintf for generating CSS strings

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Christian Stadelmann]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

Start firefox with a profile manager:
`firefox --new-instance -Profile-Manager`

This issue only happens with specific locales such as German (de-DE). If you have en_US locale, you need to start firefox like this to reproduce the bug:

` LC_ALL=de_DE firefox --new-instance -ProfileManager`


Actual results:

Gtk warnings about theme parsing errors:
> Theme parsing error: <data>:1:34: Expected ')' in color definition
> Theme parsing error: <data>:1:77: Expected ')' in color definition


Expected results:

No error message.


Additional information:
This issue is present on:
* Firefox Nightly builds from firefox-flatpak.mojefedora.cz, last build 65.0a1 (2018-12-10) (64-bit)
* Firefox 62.0-2.fc28 (Fedora 28) and any later release
* firefox-64.0-4.fc29.x86_64 (latest release on Fedora 29)
* This issue has not been present in Firefox 61, including (until) 61.0.2-3.fc28.

This issue is specific to the locale, i.e. it does not happen with the `LC_ALL=C` locale definition.

Comment 1

[Christian Stadelmann]

I did a more detailed analysis and found that the bug is caused by incorrect usage of the (locale-specific) AppendPrintf function.

According to nsTSubstring.h:701ff, AppendPrintf() is
>   /**
>    * Append a formatted string to the current string. Uses the
>    * standard printf format codes.  This uses NSPR formatting, which will be
>    * locale-aware for floating-point values.  You probably don't want to use
>    * this with floating-point values as a result.
>    */

First problematic string (value of GtkCssParser.data):
> ,000000);background-color:rgba(74,144,217,1,000000);}
which is part of a longer string (in frame #15):
> :selected{color:rgba(255,255,255,1,000000);background-color:rgba(74,144,217,1,000000);}
This in turn looks like someone mixed up different decimal separators, "," and ".". This may be caused by
/usr/src/debug/firefox-64.0-4.fc29.x86_64/widget/gtk/IMContextWrapper.cpp in mozilla::widget::SelectionStyleProvider::OnThemeChanged() in one of these lines:

>            style.AppendPrintf("color:rgba(%u,%u,%u,%f);",
>                               NS_GET_R(selectionForegroundColor),
>                               NS_GET_G(selectionForegroundColor),
>                               NS_GET_B(selectionForegroundColor), alpha);

where the alpha color is parsed with respect to the locale. This theory can be proven by running
> LC_ALL=C firefox --new-instance -ProfileManager
which does not cause the error to be printed. The fix for this bug would be to print the float in a locale-agnostic way.

The issue is also present with the Firefox Nightly build 66.0a1 (2018-12-20) (64-bit) from firefox-flatpak.mojefedora.cz.

Comment 2

[Christian Stadelmann]

Attachment: GDB backtrace from the error message

Comment 3

[Gingerbread Man]

:stransky, since you were the assignee of bug 1480760, please have a look at this if you can.

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Okay, taking this one.

Comment 5

[abc]

Same error here.
I am using firefox 64.0-1 on Arch Linux. My locale is it_CH.UTF-8.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1516101 - Don't use locale-specific AppendPrintf() to print float values, r=jhorak

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Christian, thanks for doing the legwork here!

Comment 8

[Martin Stránský [:stransky] (ni? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc105056cc2a0df64214157ba8dfc625c4f80131

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4fc0e5c9d4a
Don't use locale-specific AppendPrintf() to print float values, r=jhorak

Comment 10

[Christian Stadelmann]

(In reply to Martin Stránský [:stransky] from comment #7)

Christian, thanks for doing the legwork here!

No problem, thanks for fixing the bug ;)

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f4fc0e5c9d4a

Comment 12

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/projects/cedar/rev/f4fc0e5c9d4a8fcc6e93cff2cde0362ac04b8016
Bug 1516101 - Don't use locale-specific AppendPrintf() to print float values, r=jhorak