Details
- Type:
Bug
- Status: Open
- Priority:
Major
- Resolution: Unresolved
- Affects Version/s: odfdom-0.7.5
- Fix Version/s: None
-
- 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
- depends upon
ODFTOOLKIT-44 Remove Xerces Parser Implementation from API
- Closed
Activity
- All
- Work Log
- History
- Activity
- Transitions
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
because the patch is based on the changeset 23, so it still need code merge.
Merge the code and push it.
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.
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
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++)
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
reassign to myself
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
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)
// 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)) )
else
{
// register new automatic style
OdfOfficeAutomaticStyles automatic_styles = getAutomaticStyles();
if( automatic_styles != null )
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.
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
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); //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.
Changed target to 0.7.5, etc.
Changed summary to make the issue more obvious..
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
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)
After this fix, indexing methods of used styles work again (see test case).
Test case to add to org.odftoolkit.odfdom.doc.PresentationTest:
@Test); }
catch (Exception e){ e.printStackTrace(); Assert.fail(e.getMessage()); }
}
Additionally we should consider to implement OdfStyleablePresentationElement.setAttributeNodeNS in order to have it change the mStyleNameAttrib member.
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.
I'm OK with the patch. Please push it, Steffen.
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.
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 () 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.
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.)
Daisy and Tony, since you both already supported me with this bug, what's your opinion?
Created an attachment (id=88)
workaround: no optimization for presentation documents
Suggestion: add this workaround to 0.7, fix bug for 0.8.
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?
DrawFrameElement's mFamily is defaultly set to "graphic".
You can see "
public OdfStyleablePresentationElement(OdfFileDom ownerDocument, OdfName name)
".
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.
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.
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.
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.
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.
Created an attachment (id=86)
ODP file created by program
Created an attachment (id=85)
Program that creates and saves a new OdfPresentation Document
Verified status of this issue
2.not sure - I am not sure about this issue, maybe still need to be opened