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

RESOLVED FIXED in Firefox 66

Status

()

defect
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: moz, Assigned: stransky)

Tracking

65 Branch
mozilla66
Unspecified
Linux
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

6 months ago
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.
Reporter

Comment 1

6 months ago
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 3

6 months ago
:stransky, since you were the assignee of bug 1480760, please have a look at this if you can.
Component: Untriaged → Widget: Gtk
Flags: needinfo?(stransky)
OS: Unspecified → Linux
Product: Firefox → Core
Assignee

Comment 4

6 months ago
Okay, taking this one.
Assignee: nobody → stransky
Flags: needinfo?(stransky)

Comment 5

6 months ago
Same error here.
I am using firefox 64.0-1 on Arch Linux. My locale is it_CH.UTF-8.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee

Comment 7

5 months ago
Christian, thanks for doing the legwork here!
Assignee

Updated

5 months ago
Keywords: checkin-needed

Comment 9

5 months ago

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

Keywords: checkin-needed
Reporter

Comment 10

5 months ago

(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

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
https://hg.mozilla.org/projects/cedar/rev/f4fc0e5c9d4a8fcc6e93cff2cde0362ac04b8016
Bug 1516101 - Don't use locale-specific AppendPrintf() to print float values, r=jhorak
You need to log in before you can comment on or make changes to this bug.