Closed (works as designed)
Project:
Vertical Tabs
Version:
6.x-1.0-rc1
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
17 May 2010 at 14:19 UTC
Updated:
7 Mar 2011 at 17:09 UTC
Jump to comment: Most recent file
Comments
Comment #1
soyarma commentedI've encountered this with a Portuguese page as well.
What occurs is that in languages which use commas as decimal points the 'jQuery.extend(Drupal.settings, {' section ends up with weights like this:
"menu":{"name":"Menu settings","weight":-1,999,"callback":"menu","args":[]},
This breaks all JS on the page.
I believe that the commas are put in place by PHP's locale settings. The solution is to not use decimals in the weights, but to multiply the weight itself to give the increment desired.
On line 351 of vertical_tabs.module, change
to
I have also attached a patch.
Comment #2
dave reidNo, that means something is going horribly wrong and is changing output of all number variables everywhere.
Comment #3
dave reidYou've got something maliciously changing setlocale() and altering all PHP variables that are numbers. That will cause several problems throughout your entire install.
Comment #4
soyarma commentedIt's not malicious, it is based on the locale that PHP is being told it is in. If I tell PHP I am in a locale where they use commas for decimal points, then when number_format() (and other math functions I'd imagine) is run, it will put commas in those weights.
Somewhere in Drupal (core or contrib) some number processing is being done on weights that is being affected by a setlocale. The reason I know that it must be some signal from the CMS to PHP is that this occurs on a multilingual site. A page on the site in English or Spanish does not have the issue, but a page (a translated node from an English source, for example) does have the issue.
Either way, it makes logical sense to simply only use whole numbers for weights. Having one module (that I know of) buck the trend means we have to plan for a whole other raft of contingencies.
Comment #5
dave reidYou should not be telling PHP at all what locale you're in. That's for *Drupal* only to handle. This will also cause problems if you ever have tables that have a float field with a default value of something like '0.5' because Drupal will try to insert the value as
0,5which causes SQL problems.Comment #6
dave reidSee also #614124: Bootstrap should reset locale settings
Comment #7
soyarma commentedSorry, I didn't mean that *I* am telling PHP what locale I'm in, I meant that if something ('I' being my example) tells PHP that.
It's very odd that in searching through my entire Drupal install I cannot find anything other than unicode.inc running setlocale (with the exception of the devel module using LC_NUMERIC, C).
Be that as it may, one node that is in English is fine and the next one in Portuguese is changing it's numbers to have commas as decimals. This means it is something in Drupal making the change.
Comment #8
soyarma commentedHey Dave;
It does seem like this is really just a symptom of a larger issue. I tried the patch in the post you linked to to no avail. I figured it wouldn't help since if it is a module messing things up it is happening after bootstrap.
Interestingly enough I found that enabling the devel module and performance and then enabling query collection does affect the performance of my portuguese pages a lot. Which tells me that you were right on the money when you said I'd have other problems elsewhere.
After pulling down the exact files on my server (in case SVN was missing something that somehow got pushed live) I found that nothing uses setlocale yet somehow my portuguese pages were having this issue.
To that end I created a module that hooks in various places and sets the LC_NUMERIC to C.
This does the trick and fixes both the js writing problem as well as the slowdowns my PT pages were having from some floats having commas in them.
It is arguably a kluge, but rather than searching through dozens (or 100+ module) to find the culprit and then find it is a module you need and don't want to fork, I think this module will save people a lot of pain.
Comment #9
unegro commentedThanks, this patch really works!!