Re: svn commit: r1338836 - in /ofbiz/branches/release12.04: ./ framework/base/dtd/ framework/base/src/org/ofbiz/base/util/ framework/common/config/ framework/webtools/src/org/ofbiz/webtools/labelmanager/

Wed, 16 May 2012 01:21:26 -0700

I tried to explain it to you in the Jira issue and in my initial response to this commit. Clearly, you are not understanding the problem. If you don't understand the problem, then you shouldn't be committing half-baked "solutions." 

I will try to explain it again:
1. CommonEmptyHeader exists as a workaround to a flaw in the screen widget rendering. The simple fact that a single space character - which never needs to be translated - is in the UI label file makes it obvious that it is a hack.
2. Label Manager removes the single space from CommonEmptyHeader while saving changes to the file. That is because a stylesheet is used to format the XML, and the stylesheet removes the space.
3. The correct solution to this problem (#2) is to fix the screen widget rendering (#1).
4. Disabling space removal in the stylesheet effectively disables the entire stylesheet. The whole point of the method you modified is to strip all spaces from the XML, and then indent the XML with the specified spacing. If you turn off the space removal then the XML will not be indented properly. 

I can't make it any simpler than that.

In case you missed it in my previous replies:

-1 on this commit and the others like it.
If you can't understand the problem, then that's fine - revert the code and unassign yourself from the Jira issue so someone more qualified can fix it. 

-Adrian

On 5/16/2012 9:05 AM, Jacques Le Roux wrote:
Why don't you answer on my proposition? Why don't you *clearly explain* why this change is unnecessary instead of yelling?
I really want to fix this bug, wich removes the xml:space="preserve" attribute from CommonEmptyHeader 

Jacques

From: "Adrian Crum" <adrian.c...@sandglass-software.com>

I repeat: THIS CHANGE IS UNNECESSARY.

Please revert it.

And in case you missed it the first time:

-1 on this commit and the others like it.

-Adrian

On 5/16/2012 8:28 AM, Jacques Le Roux wrote:

I don't agree, it does not hide a problem, it solves a problem.
The (keepSpace=true) version is only used in SaveLabelsToXmlFile.saveLabelsToXmlFile(), nothing else is changed functionally. 

Would you not agree with this patch?
Index: framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java 
===================================================================
--- framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java (revision 1338984) +++ framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java (working copy) 
@@ -113,6 +113,7 @@
resourceElem.setAttribute("xmlns:xsi", "http://www.w3.org/2001/XMLSchema-instance";); resourceElem.setAttribute("xsi:noNamespaceSchemaLocation","http://ofbiz.apache.org/dtds/ofbiz-properties.xsd";); 
                for (String labelKey : labelsList) {
+                    boolean keepSpaces = false;
                    LabelInfo labelInfo = labels.get(labelKey);
if (!(labelInfo.getFileName().equalsIgnoreCase(fileName))) { 
                        continue;
@@ -136,6 +137,7 @@
valueElem.setAttribute("xml:lang", localeFound); 
                            if (valueString.trim().isEmpty()) {
valueElem.setAttribute("xml:space", "preserve"); 
+                                keepSpaces = true;
                            }
if (UtilValidate.isNotEmpty(labelValue.getLabelComment())) { 
                                Comment labelComment =
resourceDocument.createComment(StringEscapeUtils.unescapeHtml(labelValue.getLabelComment())); 
@@ -149,7 +151,7 @@
                        if (apacheLicenseText != null) {
                            fos.write(apacheLicenseText.getBytes());
                        }
- UtilXml.writeXmlDocument(resourceElem, fos, "UTF-8", !(apacheLicenseText == null), true, 4, true); + UtilXml.writeXmlDocument(resourceElem, fos, "UTF-8", !(apacheLicenseText == null), true, 4, keepSpaces); 
                    } finally {
                        fos.close();
// clear cache to see immediately the new labels and 

Jacques

From: "Adrian Crum" <adrian.c...@sandglass-software.com>
The bad thing about this change is that it is not necessary. This change hides a problem - it does not solve it. I mentioned that 
in the Jira issue.
The method is supposed to pretty-print an XML document, and if you turn off space removal, the indentation will be wrong. In other 
words, it will not be pretty.

-1 on this change in any version.

-Adrian

On 5/16/2012 2:25 AM, Scott Gray wrote:
I see in your subsequent commits that you encountered the problems caused by this type of change.
They're also a pretty good indication that once again you didn't compile or test before back-porting, I'm not sure what I can do to make the need for this any clearer to you. Please remember your responsibilities as a committer. 

Regards
Scott

On 16/05/2012, at 1:17 PM, Scott Gray wrote:
You've changed the signature on the UtilXml methods, that should not be done and especially not be back-ported to the branches. Even in the trunk the correct thing to do is to add a new method with the new signature and then (if needed) deprecate the old 
method.  Obviously deprecation shouldn't be back-ported.
There's nothing new in this comment Jacques, the general rule of thumb is never change a method signature unless it is private. 

Regards
Scott

On 16/05/2012, at 7:11 AM, jler...@apache.org wrote:


Author: jleroux
Date: Tue May 15 19:11:13 2012
New Revision: 1338836

URL: http://svn.apache.org/viewvc?rev=1338836&view=rev
Log:
"Applied fix from trunk for revision: 1338831" (conflict in CommonUiLabels.xml handled by hand) ------------------------------------------------------------------------ r1338831 | jleroux | 2012-05-15 21:03:26 +0200 (mar., 15 mai 2012) | 14 lines
Fixes https://issues.apache.org/jira/browse/OFBIZ-4652 "The Label Manager is wrongly overriding CommonEmptyHeader" * Adds a keepSpace boolean to UtilXml.writeXmlDocument(), this allows to use xsl:preserve-space into 
UtilXml.createOutputTransformer()
* Uses it into SaveLabelsToXmlFile.saveLabelsToXmlFile()
* Adds some French labels into CommonUiLabels.xml using Labels Manager to test the new functionality 
* Adds the xml:space attribute into the valueType complexType
* Adds the ofbiz-properties.xsd schema into the base-catalog.xml

I got an issue when 1st trying to commit:
Commit failed (details follow):
While preparing
'D:\workspace\ofbizClean\framework\common\config\CommonUiLabels.xml' for commit 
Inconsistent line ending style

So I forced the EOLs to my locale platform value (Win XP)
------------------------------------------------------------------------ 


Modified:
   ofbiz/branches/release12.04/   (props changed)
   ofbiz/branches/release12.04/framework/base/dtd/base-catalog.xml
ofbiz/branches/release12.04/framework/base/dtd/ofbiz-properties.xsd ofbiz/branches/release12.04/framework/base/src/org/ofbiz/base/util/UtilXml.java ofbiz/branches/release12.04/framework/common/config/CommonUiLabels.xml ofbiz/branches/release12.04/framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java 

Propchange: ofbiz/branches/release12.04/
------------------------------------------------------------------------------ 
Merged /ofbiz/trunk:r1338831
Modified: ofbiz/branches/release12.04/framework/base/dtd/base-catalog.xml 
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/base/dtd/base-catalog.xml?rev=1338836&r1=1338835&r2=1338836&view=diff ============================================================================== --- ofbiz/branches/release12.04/framework/base/dtd/base-catalog.xml (original) +++ ofbiz/branches/release12.04/framework/base/dtd/base-catalog.xml Tue May 15 19:11:13 2012 
@@ -29,4 +29,5 @@ under the License.
<system systemId="http://ofbiz.apache.org/dtds/jndi-config.xsd"; uri="jndi-config.xsd"/> <system systemId="http://ofbiz.apache.org/dtds/ofbiz-component.xsd"; uri="ofbiz-component.xsd"/> <system systemId="http://ofbiz.apache.org/dtds/ofbiz-containers.xsd"; uri="ofbiz-containers.xsd"/> +<system systemId="http://ofbiz.apache.org/dtds/ofbiz-properties.xsd"; uri="ofbiz-properties.xsd"/> 
</catalog>
Modified: ofbiz/branches/release12.04/framework/base/dtd/ofbiz-properties.xsd 
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/base/dtd/ofbiz-properties.xsd?rev=1338836&r1=1338835&r2=1338836&view=diff ============================================================================== --- ofbiz/branches/release12.04/framework/base/dtd/ofbiz-properties.xsd (original) +++ ofbiz/branches/release12.04/framework/base/dtd/ofbiz-properties.xsd Tue May 15 19:11:13 2012 
@@ -42,6 +42,7 @@ under the License.
<xs:simpleContent>
<xs:extension base="xs:string">
<xs:attribute ref="xml:lang"/>
+<xs:attribute ref="xml:space"/>
</xs:extension>
</xs:simpleContent>
</xs:complexType>
Modified: ofbiz/branches/release12.04/framework/base/src/org/ofbiz/base/util/UtilXml.java 
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/base/src/org/ofbiz/base/util/UtilXml.java?rev=1338836&r1=1338835&r2=1338836&view=diff ============================================================================== --- ofbiz/branches/release12.04/framework/base/src/org/ofbiz/base/util/UtilXml.java (original) +++ ofbiz/branches/release12.04/framework/base/src/org/ofbiz/base/util/UtilXml.java Tue May 15 19:11:13 2012 
@@ -158,20 +158,26 @@ public class UtilXml {

    // ----- TrAX Methods ----------------- //
- /** Creates a JAXP TrAX Transformer suitable for pretty-printing an - * XML document. This method is provided as an alternative to the - * deprecated<code>org.apache.xml.serialize.OutputFormat</code> class. 
-     * @param encoding Optional encoding, defaults to UTF-8
- * @param omitXmlDeclaration If<code>true</code> the xml declaration 
-     * will be omitted from the output
- * @param indent If<code>true</code>, the output will be indented - * @param indentAmount If<code>indent</code> is<code>true</code>, 
-     * the number of spaces to indent. Default is 4.
+    /**
+ * Creates a JAXP TrAX Transformer suitable for pretty-printing an XML document. This method is provided as an alternative 
to the deprecated
+     *<code>org.apache.xml.serialize.OutputFormat</code>  class.
+     *
+     * @param encoding
+     *            Optional encoding, defaults to UTF-8
+     * @param omitXmlDeclaration
+ * If<code>true</code> the xml declaration will be omitted from the output 
+     * @param indent
+     *            If<code>true</code>, the output will be indented
+     * @param indentAmount
+ * If<code>indent</code> is<code>true</code>, the number of spaces to indent. Default is 4. 
+     * @param keepSpace
+ * If<code>true</code> spaces are preserved, else striped 
     * @return A<code>Transformer</code>  instance
* @see<a href="http://java.sun.com/javase/6/docs/api/javax/xml/transform/package-summary.html";>JAXP TrAX</a> 
     * @throws TransformerConfigurationException
     */
- public static Transformer createOutputTransformer(String encoding, boolean omitXmlDeclaration, boolean indent, int 
indentAmount) throws TransformerConfigurationException {
+ public static Transformer createOutputTransformer(String encoding, boolean omitXmlDeclaration, boolean indent, int 
indentAmount, boolean keepSpace)
+            throws TransformerConfigurationException {
        StringBuilder sb = new StringBuilder();
        sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
sb.append("<xsl:stylesheet xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\"; 
xmlns:xalan=\"http://xml.apache.org/xslt\"; version=\"1.0\">\n");
@@ -189,7 +195,11 @@ public class UtilXml {
            sb.append(indentAmount<= 0 ? 4 : indentAmount);
            sb.append("\"");
        }
-        sb.append("/>\n<xsl:strip-space elements=\"*\"/>\n");
+        if (keepSpace) {
+ sb.append("/>\n<xsl:preserve-space elements=\"*\"/>\n"); 
+        } else {
+            sb.append("/>\n<xsl:strip-space elements=\"*\"/>\n");
+        }
        sb.append("<xsl:template match=\"@*|node()\">\n");
sb.append("<xsl:copy><xsl:apply-templates select=\"@*|node()\"/></xsl:copy>\n"); 
        sb.append("</xsl:template>\n</xsl:stylesheet>\n");
@@ -212,21 +222,29 @@ public class UtilXml {
        transformer.transform(source, result);
    }
- /** Serializes a DOM<code>Node</code> to an<code>OutputStream</code> 
-     * using JAXP TrAX.
-     * @param node The<code>Node</code>  to serialize
-     * @param os The<code>OutputStream</code>  to serialize to
-     * @param encoding Optional encoding, defaults to UTF-8
- * @param omitXmlDeclaration If<code>true</code> the xml declaration 
-     * will be omitted from the output
- * @param indent If<code>true</code>, the output will be indented - * @param indentAmount If<code>indent</code> is<code>true</code>, 
-     * the number of spaces to indent. Default is 4.
+    /**
+ * Serializes a DOM<code>Node</code> to an<code>OutputStream</code> using JAXP TrAX. 
+     *
+     * @param node
+     *            The<code>Node</code>  to serialize
+     * @param os
+     *            The<code>OutputStream</code>  to serialize to
+     * @param encoding
+     *            Optional encoding, defaults to UTF-8
+     * @param omitXmlDeclaration
+ * If<code>true</code> the xml declaration will be omitted from the output 
+     * @param keepSpace
+ * If<code>true</code> spaces are preserved, else striped 
+     * @param indent
+     *            If<code>true</code>, the output will be indented
+     * @param indentAmount
+ * If<code>indent</code> is<code>true</code>, the number of spaces to indent. Default is 4. * @see<a href="http://java.sun.com/javase/6/docs/api/javax/xml/transform/package-summary.html";>JAXP TrAX</a> 
     * @throws TransformerException
     */
- public static void writeXmlDocument(Node node, OutputStream os, String encoding, boolean omitXmlDeclaration, boolean 
indent, int indentAmount) throws TransformerException {
- Transformer transformer = createOutputTransformer(encoding, omitXmlDeclaration, indent, indentAmount); + public static void writeXmlDocument(Node node, OutputStream os, String encoding, boolean omitXmlDeclaration, boolean 
indent, int indentAmount,
+            boolean keepSpace) throws TransformerException {
+ Transformer transformer = createOutputTransformer(encoding, omitXmlDeclaration, indent, indentAmount, keepSpace); 
        transformDomDocument(transformer, node, os);
    }

@@ -325,9 +343,9 @@ public class UtilXml {
            return;
        }
// OutputFormat defaults are: indent on, indent = 4, include XML declaration, 
-        // charset = UTF-8, line width = 72
+        // charset = UTF-8, line width = 72, strip spaces
        try {
-            writeXmlDocument(node, os, "UTF-8", false, true, 4);
+ writeXmlDocument(node, os, "UTF-8", false, true, 4, false); 
        } catch (TransformerException e) {
            // Wrapping this exception for backwards compatibility
            throw new IOException(e.getMessage());
Modified: ofbiz/branches/release12.04/framework/common/config/CommonUiLabels.xml 
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/common/config/CommonUiLabels.xml?rev=1338836&r1=1338835&r2=1338836&view=diff ============================================================================== --- ofbiz/branches/release12.04/framework/common/config/CommonUiLabels.xml (original) +++ ofbiz/branches/release12.04/framework/common/config/CommonUiLabels.xml Tue May 15 19:11:13 2012 
@@ -8,6 +8,7 @@
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
+
    http://www.apache.org/licenses/LICENSE-2.0

    Unless required by applicable law or agreed to in writing,
@@ -17,13 +18,13 @@
    specific language governing permissions and limitations
    under the License.
-->
-<resource xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";>
+<resource xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/ofbiz-properties.xsd";> 
<property key="AP">
<value xml:lang="de">Kreditoren</value>
<value xml:lang="en">AP</value>
<value xml:lang="es">Compras</value>
<value xml:lang="fr">Cpt. fourn.</value>
-<value xml:lang="hi_IN">लेखा देय</value>
+<value xml:lang="hi-IN">लेखा देय</value>
<value xml:lang="it">AP</value>
<value xml:lang="nl">Schulden</value>
<value xml:lang="pt_BR">Contas a pagar</value>
@@ -127,6 +128,7 @@
<property key="CommonAbort">
<value xml:lang="en">Abort</value>
<value xml:lang="es">Cancelar</value>
+<value xml:lang="fr">Abandonner</value>
<value xml:lang="vi">Hủy</value>
</property>
<property key="CommonAccepted">
@@ -413,6 +415,7 @@
<value xml:lang="de">Die Adresse wurde nicht gefunden.</value>
<value xml:lang="en">Address not found.</value>
<value xml:lang="es">Dirección no encontrada.</value>
+<value xml:lang="fr">Adresse inconnue</value>
<value xml:lang="vi">Không tìm thấy địa chỉ</value>
</property>
<property key="CommonAddresses">
@@ -2770,7 +2773,8 @@
<value xml:lang="zh_TW">電子郵件</value>
</property>
<property key="CommonEmptyHeader">
-<value xml:lang="en" xml:space="preserve"> </value><!-- do not remove this! --> 
+<!-- do not  remove this! -->
+<value xml:lang="en" xml:space="preserve"> </value>
</property>
<property key="CommonEnable">
<value xml:lang="en">Enable</value>
@@ -11081,6 +11085,31 @@
<value xml:lang="zh">有效日期</value>
<value xml:lang="zh_TW">有效日</value>
</property>
+<property key="FormFieldTitle_expireButton">
+<value xml:lang="ar">نهاية الصلاحية</value>
+<value xml:lang="cs">Vypršet</value>
+<value xml:lang="de">Ablaufen</value>
+<value xml:lang="en">Expire</value>
+<value xml:lang="es">Expiración</value>
+<value xml:lang="fr">Invalider</value>
+<value xml:lang="hi-IN">निश्वासन</value>
+<value xml:lang="it">Scade</value>
+<value xml:lang="nl">Maak verlopen</value>
+<value xml:lang="pt">Expirar</value>
+<value xml:lang="pt-PT">Terminar</value>
+<value xml:lang="ro">Expira</value>
+<value xml:lang="ru">Прекратить</value>
+<value xml:lang="th">หมดเขต</value>
+<value xml:lang="vi">Hết hạn</value>
+<value xml:lang="zh">过期</value>
+<value xml:lang="zh-CN">过期</value>
+<value xml:lang="zh-TW">過期</value>
+</property>
+<property key="FormFieldTitle_extDevicePurposeActionId">
+<value xml:lang="en">External Device Purpose Action Id</value>
+<value xml:lang="es">Código acción dispositivo externo</value>
+<value xml:lang="fr">Obj. de l'action</value>
+</property>
<property key="FormFieldTitle_externalDeviceId">
<value xml:lang="en">External Device Id</value>
<value xml:lang="es">Código dispositivo externo</value>
@@ -11091,11 +11120,6 @@
<value xml:lang="es">Código tipo dispositivo externo</value>
<value xml:lang="fr">Type de Mat. Externe</value>
</property>
-<property key="FormFieldTitle_extDevicePurposeActionId">
-<value xml:lang="en">External Device Purpose Action Id</value>
-<value xml:lang="es">Código acción dispositivo externo</value>
-<value xml:lang="fr">Obj. de l'action</value>
-</property>
<property key="FormFieldTitle_fromDate">
<value xml:lang="ar">تاريخ البدء</value>
<value xml:lang="de">Von Datum</value>
Modified: ofbiz/branches/release12.04/framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java 
URL:
http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java?rev=1338836&r1=1338835&r2=1338836&view=diff ============================================================================== --- ofbiz/branches/release12.04/framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java (original) +++ ofbiz/branches/release12.04/framework/webtools/src/org/ofbiz/webtools/labelmanager/SaveLabelsToXmlFile.java Tue May 15 
19:11:13 2012
@@ -34,10 +34,8 @@ import org.ofbiz.base.util.UtilPropertie
import org.ofbiz.base.util.UtilValidate;
import org.ofbiz.base.util.UtilXml;
import org.ofbiz.base.util.cache.UtilCache;
-
import org.ofbiz.service.DispatchContext;
import org.ofbiz.service.ServiceUtil;
-
import org.w3c.dom.Comment;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -151,7 +149,7 @@ public class SaveLabelsToXmlFile {
                        if (apacheLicenseText != null) {
fos.write(apacheLicenseText.getBytes()); 
                        }
- UtilXml.writeXmlDocument(resourceElem, fos, "UTF-8", !(apacheLicenseText == null), true, 4); + UtilXml.writeXmlDocument(resourceElem, fos, "UTF-8", !(apacheLicenseText == null), true, 4, true); 
                    } finally {
                        fos.close();
// clear cache to see immediately the new labels and

Reply via email to