ODF Toolkit
  1. ODF Toolkit
  2. ODFTOOLKIT-34

Assigning shape objects the adequate style:family (graphic/presentation)

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: odfdom-0.7.5
    • Fix Version/s: None
    • Component/s: java
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: PC

      Description

      The attached code creates the attached .odp file. The date/time and footer areas are visible with background color #99ccff at the bottom left and center of the page. The weird part is that the right-hand background element (page number) isn't visible in normal view, but does show up when you turn on background view.

      It would be preferable if all three were invisible or at least had a background color of #ffffff.

        Issue Links

          Activity

          Hide
          DaLi Liu added a comment -

          Verified status of this issue
          2.not sure - I am not sure about this issue, maybe still need to be opened

          Show
          DaLi Liu added a comment - Verified status of this issue 2.not sure - I am not sure about this issue, maybe still need to be opened
          Hide
          Svante Schubert added a comment -

          There was a slight misunderstanding here, I added a new patch to summarize the thoughts within the issue, but the patch was not a final patch.

          In the future we should in general always add first a patch with the test that should be fixed and than a separate patch with the solution.

          To test first the test and next the fix.

          My patch was meant as a patch already embracing the partly fix from before, but as well the a new test that shows the problem.

          For this reason, I have reopened the issue again and made a rollback of the (unworking) test on the master.

          Most likely this issue won't be fixed for 0.8.
          Nevertheless assigned it to myself.

          Regards,
          Svante

          Show
          Svante Schubert added a comment - There was a slight misunderstanding here, I added a new patch to summarize the thoughts within the issue, but the patch was not a final patch. In the future we should in general always add first a patch with the test that should be fixed and than a separate patch with the solution. To test first the test and next the fix. My patch was meant as a patch already embracing the partly fix from before, but as well the a new test that shows the problem. For this reason, I have reopened the issue again and made a rollback of the (unworking) test on the master. Most likely this issue won't be fixed for 0.8. Nevertheless assigned it to myself. Regards, Svante
          Hide
          Wei Hua Wang added a comment -

          because the patch is based on the changeset 23, so it still need code merge.
          Merge the code and push it.

          Show
          Wei Hua Wang added a comment - because the patch is based on the changeset 23, so it still need code merge. Merge the code and push it.
          Hide
          Svante Schubert added a comment -

          Created an attachment (id=169)
          Assigning shape objects the adequate style:family (update on latest revision plus additional test)

          This is an update of the previous patch, synchronized to the latest revision and embracing the new test reflecting the potential problems found.

          In addition not the DOM classes are being checked in, but the javacodetemplate.xml to generate the source.

          If you apply the patch, you still have to create the source 'mvn -P codegen' and than only the new part of the PresentationTest will fail, marked as to be fixed in this issue.

          Show
          Svante Schubert added a comment - Created an attachment (id=169) Assigning shape objects the adequate style:family (update on latest revision plus additional test) This is an update of the previous patch, synchronized to the latest revision and embracing the new test reflecting the potential problems found. In addition not the DOM classes are being checked in, but the javacodetemplate.xml to generate the source. If you apply the patch, you still have to create the source 'mvn -P codegen' and than only the new part of the PresentationTest will fail, marked as to be fixed in this issue.
          Hide
          Svante Schubert added a comment -

          First of all the problem with the Xerces API stated below does not exist. If we inherit from the Xerces API and override methods our methods will always be called.

          Second, the problem with intercepting setAttributeNodeNS(newAttr) for automatic style counting on the element is not solvable with this approach as the to be known stylename (attribute value) is being set in a different call on the element. The solution seems be to add some style user counting in all *:style-name attributes.

          Guess, before I touch the javacodetemplate.xml again and fix this, I will wait till the patches in the queue are being reviewed and integrated...

          Regards,
          Svante

          Show
          Svante Schubert added a comment - First of all the problem with the Xerces API stated below does not exist. If we inherit from the Xerces API and override methods our methods will always be called. Second, the problem with intercepting setAttributeNodeNS(newAttr) for automatic style counting on the element is not solvable with this approach as the to be known stylename (attribute value) is being set in a different call on the element. The solution seems be to add some style user counting in all *:style-name attributes. Guess, before I touch the javacodetemplate.xml again and fix this, I will wait till the patches in the queue are being reviewed and integrated... Regards, Svante
          Hide
          Ying Chun Guo added a comment -

          After discussion with Wei Hua, to resolve this issue completely, we need make sure we invoke setAttributeNodeNS of ElementNSImpl when parsing a document and invoke setAttributeNodeNS of OdfStylableElement when setting the value of an attribute.

          ODFTOOLKIT-44 provides a way to resolve this issue. But I think there might be other way to resolve it, for example:

          Adding a method called setAttrNodeNS in OdfStylableElement,

          public Attr setAttrNodeNS(Attr newAttr)

          { return super.setAttributeNodeNS(newAttr); }

          and invoking setAttrNodeNS if the element is an instance of OdfStylableElement.

          public void startElement(...) throws SAXException {
          .....
          for (int i = 0; i < attributes.getLength(); i++)

          { OdfAttribute attr = mDocument.createAttributeNS(...); if (element instanceof OdfStylableElement) ((OdfStylableElement)element).setAttrNodeNS(attr); else element.setAttributeNodeNS(attr); ..... }
          Show
          Ying Chun Guo added a comment - After discussion with Wei Hua, to resolve this issue completely, we need make sure we invoke setAttributeNodeNS of ElementNSImpl when parsing a document and invoke setAttributeNodeNS of OdfStylableElement when setting the value of an attribute. ODFTOOLKIT-44 provides a way to resolve this issue. But I think there might be other way to resolve it, for example: Adding a method called setAttrNodeNS in OdfStylableElement, public Attr setAttrNodeNS(Attr newAttr) { return super.setAttributeNodeNS(newAttr); } and invoking setAttrNodeNS if the element is an instance of OdfStylableElement. public void startElement(...) throws SAXException { ..... for (int i = 0; i < attributes.getLength(); i++) { OdfAttribute attr = mDocument.createAttributeNS(...); if (element instanceof OdfStylableElement) ((OdfStylableElement)element).setAttrNodeNS(attr); else element.setAttributeNodeNS(attr); ..... }
          Hide
          Svante Schubert added a comment -

          Hi Daisy,

          let's assume that we already have hidden the Xerces API from the user (that ODFTOOLKIT-44 would be already fixed).
          Under this theoretical prerequisite, could one of your team verify this issue again?

          Thanks in advance,
          Svante

          Show
          Svante Schubert added a comment - Hi Daisy, let's assume that we already have hidden the Xerces API from the user (that ODFTOOLKIT-44 would be already fixed). Under this theoretical prerequisite, could one of your team verify this issue again? Thanks in advance, Svante
          Hide
          Svante Schubert added a comment -

          reassign to myself

          Show
          Svante Schubert added a comment - reassign to myself
          Hide
          Svante Schubert added a comment -

          Frank analyzed the problem you were referring, he stated that it is possible at any time to set the value of the *:style-name attribute of an stylable element any time directly on the Xerces DOM API.

          To intercept this call, we need to prevent the user of accessing this native API directly.

          Therefore this issue becomes dependent of ODFTOOLKIT-44 (Encapsulating the Xerces API).

          Daisy, is it possible that you might provide us with a patch this week?

          Thanks in advance,
          Svante

          Show
          Svante Schubert added a comment - Frank analyzed the problem you were referring, he stated that it is possible at any time to set the value of the *:style-name attribute of an stylable element any time directly on the Xerces DOM API. To intercept this call, we need to prevent the user of accessing this native API directly. Therefore this issue becomes dependent of ODFTOOLKIT-44 (Encapsulating the Xerces API). Daisy, is it possible that you might provide us with a patch this week? Thanks in advance, Svante
          Hide
          Wei Hua Wang added a comment -

          I tried to fix it, but I found another issue when I override the setAttributeNodeNS of OdfStylableElement, the code is like this:
          ****************************************************************
          /**

          • Set style attribute value with uri and name
          • @param uri The namespace uri
          • @param name The attribute name
          • @param value The attribute value
            */
            @Override
            public void setAttributeNS(String uri, String name, String value) { super.setAttributeNS(uri, name, value); checkStyleNameAttrib(uri, name, value); }

          @Override
          public Attr setAttributeNodeNS(Attr newAttr)

          { Attr returnAttr = super.setAttributeNodeNS(newAttr); String uri = newAttr.getNamespaceURI(); String localname = newAttr.getName(); //Note: getValue will cause attribute validate errors. String value = newAttr.getValue(); checkStyleNameAttrib(uri, localname,value); return returnAttr; }

          // check if style has changed
          private void checkStyleNameAttrib(String uri, String localname, String value){

          if( mStyleNameAttrib.equals(uri, name) )
          {
          OdfStyle autoStyle = null;

          // optimization: check if we already know this automatic style
          if( (mAutomaticStyle != null) && (mAutomaticStyle.getStyleNameAttribute().equals(value)) )

          { // nothing todo }

          else
          {
          // register new automatic style
          OdfOfficeAutomaticStyles automatic_styles = getAutomaticStyles();
          if( automatic_styles != null )

          { autoStyle = automatic_styles.getStyle(value, getStyleFamily()); }

          if( mAutomaticStyle != null)

          { mAutomaticStyle.removeStyleUser(this); }

          mAutomaticStyle = autoStyle;

          if( mAutomaticStyle != null )

          { mAutomaticStyle.addStyleUser(this); }

          }
          }
          }
          *************************************************
          when parsing the document, the OdfStylableElement.setAttributeNodeNS will be used when start to parse the OdfStylableElement, but the value of this attribute has not been set yet, it will be set after the setAttributeNodeNS operation, the above setAttributeNodeNS implementation will getValue of this attribute, while each attribute has also override the getValue method in order to validate the value, here will induce the validation error because the value of this attribute has not been set yet.
          I have no idea about how to solve this problem.
          @Svante and Frank, because I am now busy with the navigation API, so I will return this issue to you.

          Show
          Wei Hua Wang added a comment - I tried to fix it, but I found another issue when I override the setAttributeNodeNS of OdfStylableElement, the code is like this: **************************************************************** /** Set style attribute value with uri and name @param uri The namespace uri @param name The attribute name @param value The attribute value */ @Override public void setAttributeNS(String uri, String name, String value) { super.setAttributeNS(uri, name, value); checkStyleNameAttrib(uri, name, value); } @Override public Attr setAttributeNodeNS(Attr newAttr) { Attr returnAttr = super.setAttributeNodeNS(newAttr); String uri = newAttr.getNamespaceURI(); String localname = newAttr.getName(); //Note: getValue will cause attribute validate errors. String value = newAttr.getValue(); checkStyleNameAttrib(uri, localname,value); return returnAttr; } // check if style has changed private void checkStyleNameAttrib(String uri, String localname, String value){ if( mStyleNameAttrib.equals(uri, name) ) { OdfStyle autoStyle = null; // optimization: check if we already know this automatic style if( (mAutomaticStyle != null) && (mAutomaticStyle.getStyleNameAttribute().equals(value)) ) { // nothing todo } else { // register new automatic style OdfOfficeAutomaticStyles automatic_styles = getAutomaticStyles(); if( automatic_styles != null ) { autoStyle = automatic_styles.getStyle(value, getStyleFamily()); } if( mAutomaticStyle != null) { mAutomaticStyle.removeStyleUser(this); } mAutomaticStyle = autoStyle; if( mAutomaticStyle != null ) { mAutomaticStyle.addStyleUser(this); } } } } ************************************************* when parsing the document, the OdfStylableElement.setAttributeNodeNS will be used when start to parse the OdfStylableElement, but the value of this attribute has not been set yet, it will be set after the setAttributeNodeNS operation, the above setAttributeNodeNS implementation will getValue of this attribute, while each attribute has also override the getValue method in order to validate the value, here will induce the validation error because the value of this attribute has not been set yet. I have no idea about how to solve this problem. @Svante and Frank, because I am now busy with the navigation API, so I will return this issue to you.
          Hide
          Svante Schubert added a comment -

          Hi Weihua,

          just discussed your concern with Frank and we both agree on your suggestion!
          Do you have the fix on your machine, so you might simply update the patch with your new test and an additional OdfStylableElement method?
          If it is not possible for you to fix this, just return the issue to me and I will put it in line..
          Any help is most welcome

          Thanks for your valuable feed-back!
          Svante

          Show
          Svante Schubert added a comment - Hi Weihua, just discussed your concern with Frank and we both agree on your suggestion! Do you have the fix on your machine, so you might simply update the patch with your new test and an additional OdfStylableElement method? If it is not possible for you to fix this, just return the issue to me and I will put it in line.. Any help is most welcome Thanks for your valuable feed-back! Svante
          Hide
          Wei Hua Wang added a comment -

          I have reviewed the patch, and I agree with this approach, but the patch seems still has a defect related with the style usage count.
          **************************************************************
          //Test case to test the usage count
          public void testStyleUsageCount() {

          try

          { OdfOfficeAutomaticStyles s = odfdoc.getStylesDom().getAutomaticStyles(); OdfStyle pr1 = s.getStyle("pr1", OdfStyleFamily.Presentation); int styleUserCount = pr1.getStyleUserCount(); XPath xpath = odfdoc.getXPath(); NodeList elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); int elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); //add by weihuaWang, the bug will be induced by using set attribute method OdfDrawFrame frame = (OdfDrawFrame)elementsWithStyle.item(0); frame.setPresentationStyleNameAttribute("pr2"); styleUserCount = pr1.getStyleUserCount(); elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); }

          catch (Exception e)

          { e.printStackTrace(); Assert.fail(e.getMessage()); }

          }
          **************************************
          the test is failed because I use "frame.setPresentationStyleNameAttribute("pr2");", from this we can see that
          addStyleUser should be called not only when the style node is inserted, the style attribute value has been set, but also when the style attribute node has been set.
          So OdfStylableElement should also override setAttributeNodeNS(Attr) method and implement it the same with OdfStylableElement.setAttributeNS(String uri, String name, String value)

          Just Remind:if the patch has been applied, the optimize() method should be uncommented.

          Show
          Wei Hua Wang added a comment - I have reviewed the patch, and I agree with this approach, but the patch seems still has a defect related with the style usage count. ************************************************************** //Test case to test the usage count public void testStyleUsageCount() { try { OdfOfficeAutomaticStyles s = odfdoc.getStylesDom().getAutomaticStyles(); OdfStyle pr1 = s.getStyle("pr1", OdfStyleFamily.Presentation); int styleUserCount = pr1.getStyleUserCount(); XPath xpath = odfdoc.getXPath(); NodeList elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); int elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); //add by weihuaWang, the bug will be induced by using set attribute method OdfDrawFrame frame = (OdfDrawFrame)elementsWithStyle.item(0); frame.setPresentationStyleNameAttribute("pr2"); styleUserCount = pr1.getStyleUserCount(); elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); } catch (Exception e) { e.printStackTrace(); Assert.fail(e.getMessage()); } } ************************************** the test is failed because I use "frame.setPresentationStyleNameAttribute("pr2");", from this we can see that addStyleUser should be called not only when the style node is inserted, the style attribute value has been set, but also when the style attribute node has been set. So OdfStylableElement should also override setAttributeNodeNS(Attr) method and implement it the same with OdfStylableElement.setAttributeNS(String uri, String name, String value) Just Remind:if the patch has been applied, the optimize() method should be uncommented.
          Hide
          Svante Schubert added a comment -

          Changed target to 0.7.5, etc.

          Show
          Svante Schubert added a comment - Changed target to 0.7.5, etc.
          Hide
          Svante Schubert added a comment -

          Changed summary to make the issue more obvious..

          Show
          Svante Schubert added a comment - Changed summary to make the issue more obvious..
          Hide
          Svante Schubert added a comment -

          Created an attachment (id=120)
          Assigning shape objects the adequate style:family (graphic/presentation)

          Could someone (not Sun) review the patch, please!
          I did it once, but I would like to have officially developer and reviewer from the same company.

          David, Rob or perhaps our new active member from datanucleus?
          Won't take long and I tried to explain everything in detail!

          Thanks in advance,
          Svante

          Show
          Svante Schubert added a comment - Created an attachment (id=120) Assigning shape objects the adequate style:family (graphic/presentation) Could someone (not Sun) review the patch, please! I did it once, but I would like to have officially developer and reviewer from the same company. David, Rob or perhaps our new active member from datanucleus? Won't take long and I tried to explain everything in detail! Thanks in advance, Svante
          Hide
          Svante Schubert added a comment -

          I have already reviewed the patch (upload soon), but I would like to have it reviewed by someone outside of Sun as well.

          Before of this some ODF background knowledge about ODF shapes:

          In ODF a style (ie. style:style) is always identified not alone by its name, but as well by its style:family. Elements are in general predefined to one style:family.
          For instance, a paragraph (text) will have a style from the style:family="paragraph", a span (text:span) would have a style:family="text"
          Only ODF shapes (e.g. draw:frame) can choose between two different style:family values, ie. 'presentation' and 'graphic'.
          The idea behind is that 'graphic' family shapes have a style that belong to the document similar as all other styles, but 'presentation' family shapes have a style that belongs to a master page.
          The difference:
          Whenever in an application the master page of a page is being changed, all 'graphic' shapes look the same, only the 'presentation' shapes will get a new look-and-feel from the new master page.

          (added the explanation as JavaDoc to OdfStylablePresentationElement)

          The problem here:
          The previous fix in OdfStylablePresentationElement (now called OdfStylableShapeElement) did not work anymore as during parsing the document in the OdfDocument.startElement(..) function, the attribute is created upon:

          element.setAttributeNodeNS(attr);
          ..
          attr.setValue(attributes.getValue);

          instead as previous by:
          element.setAttributeNS(uri, localname, value);

          This was done to enable validation within an attribute class about the possible attribute values, which depend on the element context (parent).

          For this reason the overridden method in OdfStylableShapeElement did not work/was not called:

          @Override
          public void setAttributeNS(String uri, String localname, String value)
          ...

          Solution: Add the same fix for the new signature being called

          @Override
          public Attr setAttributeNodeNS(Attr newAttr)

          { String uri = newAttr.getNamespaceURI(); String localname = newAttr.getName(); adjustStyleNameAttrib(uri, localname); return super.setAttributeNodeNS(newAttr); }

          After this fix, indexing methods of used styles work again (see test case).

          Show
          Svante Schubert added a comment - I have already reviewed the patch (upload soon), but I would like to have it reviewed by someone outside of Sun as well. Before of this some ODF background knowledge about ODF shapes: In ODF a style (ie. style:style) is always identified not alone by its name, but as well by its style:family. Elements are in general predefined to one style:family. For instance, a paragraph (text ) will have a style from the style:family="paragraph", a span (text:span) would have a style:family="text" Only ODF shapes (e.g. draw:frame) can choose between two different style:family values, ie. 'presentation' and 'graphic'. The idea behind is that 'graphic' family shapes have a style that belong to the document similar as all other styles, but 'presentation' family shapes have a style that belongs to a master page. The difference: Whenever in an application the master page of a page is being changed, all 'graphic' shapes look the same, only the 'presentation' shapes will get a new look-and-feel from the new master page. (added the explanation as JavaDoc to OdfStylablePresentationElement) The problem here: The previous fix in OdfStylablePresentationElement (now called OdfStylableShapeElement) did not work anymore as during parsing the document in the OdfDocument.startElement(..) function, the attribute is created upon: element.setAttributeNodeNS(attr); .. attr.setValue(attributes.getValue ); instead as previous by: element.setAttributeNS(uri, localname, value); This was done to enable validation within an attribute class about the possible attribute values, which depend on the element context (parent). For this reason the overridden method in OdfStylableShapeElement did not work/was not called: @Override public void setAttributeNS(String uri, String localname, String value) ... Solution: Add the same fix for the new signature being called @Override public Attr setAttributeNodeNS(Attr newAttr) { String uri = newAttr.getNamespaceURI(); String localname = newAttr.getName(); adjustStyleNameAttrib(uri, localname); return super.setAttributeNodeNS(newAttr); } After this fix, indexing methods of used styles work again (see test case).
          Hide
          Frank Meies added a comment -

          Test case to add to org.odftoolkit.odfdom.doc.PresentationTest:

          @Test

          public void testStyleUsageCount() {

          try

          { OdfOfficeAutomaticStyles s = odfdoc.getStylesDom().getAutomaticStyles(); OdfStyle pr1 = s.getStyle("pr1", OdfStyleFamily.Presentation); int styleUserCount = pr1.getStyleUserCount(); XPath xpath = odfdoc.getXPath(); NodeList elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); int elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); }

          catch (Exception e)

          { e.printStackTrace(); Assert.fail(e.getMessage()); }

          }

          Show
          Frank Meies added a comment - Test case to add to org.odftoolkit.odfdom.doc.PresentationTest: @Test public void testStyleUsageCount() { try { OdfOfficeAutomaticStyles s = odfdoc.getStylesDom().getAutomaticStyles(); OdfStyle pr1 = s.getStyle("pr1", OdfStyleFamily.Presentation); int styleUserCount = pr1.getStyleUserCount(); XPath xpath = odfdoc.getXPath(); NodeList elementsWithStyle = (NodeList) xpath.evaluate("//draw:frame[@presentation:style-name='pr1']", odfdoc.getStylesDom(), XPathConstants.NODESET); int elementsWithStyleCount = elementsWithStyle.getLength(); Assert.assertTrue(styleUserCount == elementsWithStyleCount); } catch (Exception e) { e.printStackTrace(); Assert.fail(e.getMessage()); } }
          Hide
          Frank Meies added a comment -

          Additionally we should consider to implement OdfStyleablePresentationElement.setAttributeNodeNS in order to have it change the mStyleNameAttrib member.

          Show
          Frank Meies added a comment - Additionally we should consider to implement OdfStyleablePresentationElement.setAttributeNodeNS in order to have it change the mStyleNameAttrib member.
          Hide
          Frank Meies added a comment -

          Well, I don't consider this issue fixed. Tony gave a good analysis of the problem: getStyleUserCount() for style "pr1" is 0, although there are three draw:frame elements with presentation:style-name set to pr1. The reason is that in OdfStyleableElement.onInsertNode the automatic style for this element is not found So I suggest to fix the root cause of this issue:

          Let's have a look at the OdfStyleableElement class. There's a member mStyleNameAttrib that specifies the appropriate style-name attribute for this element. For a OdfStyleablePresentationElement this can be either draw:style-name or presentation:style-name. The default is draw:style-name. In OdfStyleablePresentationElement.setAttributeNS the mStyleNameAttrib is changed if either a draw:style-name or a presentation:style-name attribute is set. But when creating the draw:frame element in OdfDocument.startElement, we see that the attributes are not set using setAttributeNS, instead a new OdfAttribute is created and set using setAttributeNode. Then the attribute value is set. This order is important because the value set of an attribute can depend on the context of the styleable element. So in my opinion we have to change the setValue method of the PresentationStyleNameAttribut and the DrawStyleNameAttribut to change the mStyleNameAttrib member of their owner node. Does this make sense?

          By the way: Having a working optimize() method is highly desirable. Otherwise you will end up having a lot of duplicate automatic styles: Create 100 paragraphs and apply the bold attribute to each one of it. The result is that your file will contain 100 identical automatic styles.

          Show
          Frank Meies added a comment - Well, I don't consider this issue fixed. Tony gave a good analysis of the problem: getStyleUserCount() for style "pr1" is 0, although there are three draw:frame elements with presentation:style-name set to pr1. The reason is that in OdfStyleableElement.onInsertNode the automatic style for this element is not found So I suggest to fix the root cause of this issue: Let's have a look at the OdfStyleableElement class. There's a member mStyleNameAttrib that specifies the appropriate style-name attribute for this element. For a OdfStyleablePresentationElement this can be either draw:style-name or presentation:style-name. The default is draw:style-name. In OdfStyleablePresentationElement.setAttributeNS the mStyleNameAttrib is changed if either a draw:style-name or a presentation:style-name attribute is set. But when creating the draw:frame element in OdfDocument.startElement, we see that the attributes are not set using setAttributeNS, instead a new OdfAttribute is created and set using setAttributeNode. Then the attribute value is set. This order is important because the value set of an attribute can depend on the context of the styleable element. So in my opinion we have to change the setValue method of the PresentationStyleNameAttribut and the DrawStyleNameAttribut to change the mStyleNameAttrib member of their owner node. Does this make sense? By the way: Having a working optimize() method is highly desirable. Otherwise you will end up having a lot of duplicate automatic styles: Create 100 paragraphs and apply the bold attribute to each one of it. The result is that your file will contain 100 identical automatic styles.
          Hide
          steffeng added a comment -

          closed as fixed.

          Show
          steffeng added a comment - closed as fixed.
          Hide
          steffeng added a comment -

          bug set to fixed.

          Show
          steffeng added a comment - bug set to fixed.
          Hide
          Ying Chun Guo added a comment -

          I'm OK with the patch. Please push it, Steffen.

          Show
          Ying Chun Guo added a comment - I'm OK with the patch. Please push it, Steffen.
          Hide
          steffeng added a comment -

          Created an attachment (id=89)
          Workaround: commented optimize() in OdfDocument

          I agree. The attached patch comments optimize() that is done automatically when a document is saved.
          Users may still do optimization by hand if they call optimize() on the OdfOfficeAutomaticStyles class, as the org/odftoolkit/odfdom/doc/StyleTest.java does. If there is another bug in optimize(), we may find it that way.

          If you agree with that, just notify me and I will push the workaround.

          Show
          steffeng added a comment - Created an attachment (id=89) Workaround: commented optimize() in OdfDocument I agree. The attached patch comments optimize() that is done automatically when a document is saved. Users may still do optimization by hand if they call optimize() on the OdfOfficeAutomaticStyles class, as the org/odftoolkit/odfdom/doc/StyleTest.java does. If there is another bug in optimize(), we may find it that way. If you agree with that, just notify me and I will push the workaround.
          Hide
          Ying Chun Guo added a comment -

          Tony tested ODF graphics, chart, text and spreadsheet documents today, and he didn't find any problems.
          But Tony and I agree with David. We prefer to remove the automatic styles optimization for all kinds of documents temporarily in 0.7, because we are not sure if the optimization will cause any other errors. ODFTOOLKIT-27 (https://odftoolkit.org/bugzilla/show_bug.cgi?id=34) was caused by optimization too. And the optimization is an advance feature of styles, not a basic function, right? If we remove this feature, it won't affect the function, but it will improve the stability. So I think it is worthy.

          Show
          Ying Chun Guo added a comment - Tony tested ODF graphics, chart, text and spreadsheet documents today, and he didn't find any problems. But Tony and I agree with David. We prefer to remove the automatic styles optimization for all kinds of documents temporarily in 0.7, because we are not sure if the optimization will cause any other errors. ODFTOOLKIT-27 ( https://odftoolkit.org/bugzilla/show_bug.cgi?id=34 ) was caused by optimization too. And the optimization is an advance feature of styles, not a basic function, right? If we remove this feature, it won't affect the function, but it will improve the stability. So I think it is worthy.
          Hide
          J David Eisenberg added a comment -

          I would recommend no optimization at all in 0.7. We have evidence of a problem with optimization in presentations, but that bug may cause other problems with other file types, and we just have not encountered them yet. For example, would an .odg file have problems? (I haven't tested it to find out.)

          Show
          J David Eisenberg added a comment - I would recommend no optimization at all in 0.7. We have evidence of a problem with optimization in presentations, but that bug may cause other problems with other file types, and we just have not encountered them yet. For example, would an .odg file have problems? (I haven't tested it to find out.)
          Hide
          steffeng added a comment -

          Daisy and Tony, since you both already supported me with this bug, what's your opinion?

          Show
          steffeng added a comment - Daisy and Tony, since you both already supported me with this bug, what's your opinion?
          Hide
          steffeng added a comment -

          Created an attachment (id=88)
          workaround: no optimization for presentation documents

          Suggestion: add this workaround to 0.7, fix bug for 0.8.

          Show
          steffeng added a comment - Created an attachment (id=88) workaround: no optimization for presentation documents Suggestion: add this workaround to 0.7, fix bug for 0.8.
          Hide
          steffeng added a comment -

          Since I cannot find where the bug lies at the moment, I plead for a workaround: no optimization for presentation documents, until the bug is fixed.

          Any opinions?

          Show
          steffeng added a comment - Since I cannot find where the bug lies at the moment, I plead for a workaround: no optimization for presentation documents, until the bug is fixed. Any opinions?
          Hide
          tony wei added a comment -

          DrawFrameElement's mFamily is defaultly set to "graphic".
          You can see "
          public OdfStyleablePresentationElement(OdfFileDom ownerDocument, OdfName name)

          { super( ownerDocument, name, OdfStyleFamily.Graphic, DrawStyleAttrName ); }

          ".
          But in style.xml the "pr1" styleFamily is "presentation", the optimize() can't find the style "presentation, pr1" in draw:frame of content.xml. There are only "graphic, pr1". That will make "OdfStyle.getStyleUserCount" return zero and "pr1" is lost.

          Show
          tony wei added a comment - DrawFrameElement's mFamily is defaultly set to "graphic". You can see " public OdfStyleablePresentationElement(OdfFileDom ownerDocument, OdfName name) { super( ownerDocument, name, OdfStyleFamily.Graphic, DrawStyleAttrName ); } ". But in style.xml the "pr1" styleFamily is "presentation", the optimize() can't find the style "presentation, pr1" in draw:frame of content.xml. There are only "graphic, pr1". That will make "OdfStyle.getStyleUserCount" return zero and "pr1" is lost.
          Hide
          J David Eisenberg added a comment -

          Just a note: the src/main/resources/OdfPresentationDocument.odp file is 6882 bytes. The output of the program (with optimization) is 7319 bytes. I commented out the body of OdfDocument.optimize() and rebuilt odfdom.jar, and then ran the test program again. The resulting ODP file is 7409 bytes. That's 1% larger.

          If a solution can't be found before release of 0.7, I would suggest not doing optimization. For programs that delete a lot of content from an existing document, the result would have "too many" automatic styles and be larger than it needs to be.

          How much larger might a file become? As an experiment of an extreme case, I unzipped the output file, and copied-and-pasted the content of the <office:automatic-styles> element in styles.xml ten times. That adds an extra 100 "unused" styles to the file, more than doubling the size of the styles.xml file. I then re-zipped the output file, and it added 4215 bytes to the total file size.

          Show
          J David Eisenberg added a comment - Just a note: the src/main/resources/OdfPresentationDocument.odp file is 6882 bytes. The output of the program (with optimization) is 7319 bytes. I commented out the body of OdfDocument.optimize() and rebuilt odfdom.jar, and then ran the test program again. The resulting ODP file is 7409 bytes. That's 1% larger. If a solution can't be found before release of 0.7, I would suggest not doing optimization. For programs that delete a lot of content from an existing document, the result would have "too many" automatic styles and be larger than it needs to be. How much larger might a file become? As an experiment of an extreme case, I unzipped the output file, and copied-and-pasted the content of the <office:automatic-styles> element in styles.xml ten times. That adds an extra 100 "unused" styles to the file, more than doubling the size of the styles.xml file. I then re-zipped the output file, and it added 4215 bytes to the total file size.
          Hide
          tony wei added a comment -

          I did some invetigation on it. I found the style "pr1" is optimized during OdfDocument.optimize(). The OdfStylableElement.getStyleName() return "null" for "pr1" style. I suspect the stylefamily with presentation namespace has some bug.

          Also I found the OdfStyleFamily.Presentation short draw:fill-color and fo:min-height properties. I will do more investigation on it.

          Show
          tony wei added a comment - I did some invetigation on it. I found the style "pr1" is optimized during OdfDocument.optimize(). The OdfStylableElement.getStyleName() return "null" for "pr1" style. I suspect the stylefamily with presentation namespace has some bug. Also I found the OdfStyleFamily.Presentation short draw:fill-color and fo:min-height properties. I will do more investigation on it.
          Hide
          steffeng added a comment -

          sorry, wrong bug!

          Show
          steffeng added a comment - sorry, wrong bug!
          Hide
          steffeng added a comment -

          .

          Show
          steffeng added a comment - .
          Hide
          steffeng added a comment -

          I will have a look into it.

          Show
          steffeng added a comment - I will have a look into it.
          Hide
          Ying Chun Guo added a comment -

          I might not describe it clearly.

          The bug that David reported is that: he created a document by OdfPresentation.newPresentationDocument(), and saved the document. But the new document is not as same as the ODF presentation template "OdfPresentationDocument.odp" under src\main\resources. Some styles are missed in style.xml, and the date/time and footer areas are visible in the new document, while they are not visible in the old template document.

          Show
          Ying Chun Guo added a comment - I might not describe it clearly. The bug that David reported is that: he created a document by OdfPresentation.newPresentationDocument(), and saved the document. But the new document is not as same as the ODF presentation template "OdfPresentationDocument.odp" under src\main\resources. Some styles are missed in style.xml, and the date/time and footer areas are visible in the new document, while they are not visible in the old template document.
          Hide
          Ying Chun Guo added a comment -

          It's an interesting problem.
          I checked the file entries of ODF presentation, and found the reason was some automatically styles defined in style.xml was deleted when invoking OdfDocument.optimize().
          These deleted automatically styles are pr1, pr2 and pr3. It might cased by: when style.xml is parsing to XML dom, mStyleUser in OdfStyleBase is not set a proper value but set to zero. So after automatical styles are optimized, these three styles are deleted.
          Assign to Steffen to have a further debug.

          Show
          Ying Chun Guo added a comment - It's an interesting problem. I checked the file entries of ODF presentation, and found the reason was some automatically styles defined in style.xml was deleted when invoking OdfDocument.optimize(). These deleted automatically styles are pr1, pr2 and pr3. It might cased by: when style.xml is parsing to XML dom, mStyleUser in OdfStyleBase is not set a proper value but set to zero. So after automatical styles are optimized, these three styles are deleted. Assign to Steffen to have a further debug.
          Hide
          J David Eisenberg added a comment -

          Created an attachment (id=86)
          ODP file created by program

          Show
          J David Eisenberg added a comment - Created an attachment (id=86) ODP file created by program
          Hide
          J David Eisenberg added a comment -

          Created an attachment (id=85)
          Program that creates and saves a new OdfPresentation Document

          Show
          J David Eisenberg added a comment - Created an attachment (id=85) Program that creates and saves a new OdfPresentation Document

            People

            • Assignee:
              Svante Schubert
              Reporter:
              J David Eisenberg
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:

                Development