Bugzilla – Full Text Bug Listing
| Summary: | ArrayIndexOutOfBoundException is thrown while editing HTML model | ||
|---|---|---|---|
| Product: | [WebTools] WTP Source Editing | Reporter: | Masabumi Koinuma <koinuma> |
| Component: | wst.html | Assignee: | David Williams <david_williams> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | ||
| Version: | 1.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
I'm able to reproduce this (easily ... thanks for the good test cases!) and have looked at in some detail. There's basically something deeply wrong with the "new model" logic. (that is, if whole document's contents are replaced, all is supposed to be reinitialized afresh, immediately). As far as I can tell, this has likely been broken for a while. Guess we seldom use "set" once we have a model. I suspect any fix would be too "deep" to make for Release 1 (that is, introduce risk of regression). So, I'm wondering, for your immeidate needs, do you need to use "set"? Or, would it suffice to use "replace" ... replacing, natucally, all the existing text in the structured document? At this point, I'm hoping this would only cause some inefficiencies, if a large document's text was replaced instead of set. As far as I can tell, other aspects of the document (content model, etc) would be updated appropriately. Let me know if there's some specific thing you were trying to accomplish, or if this was just a matter of performance gains. Thanks again for the good test case. There is no reason for me to keep using set(). I can replace my codes to replace() from set() safely. But a question comes up to my mind. Why do you have separate implementations for replace() and set()? It looks the implementation for set() would be enough for just calling replace() with entire range of the document, wouldn't it? Is it safe fix on your side? An excellent idea. I checked the spec of IDocument's set(String) method, which we want to "parallel" and its quite explicit "This method is a convenience method for replace(0, getLength(), text)." I've checked and there is currently nothing "special" we are currently doing with the "new Document Event" so there would be no implact. There would be a slight API change ... well ... technically, its not an API yet, but an important provisional one. the return type of setText would now have to be changed to StructuredDocumentEvent. This still leaves open the future possibility of performance enhancements by doing something special on 'set' vs. 'replace'. For the record, I also added (and changed) some JUnit tests to better test and capture any changes in the future. Some failed with "old" code, but work fine with new code). Fix will be in RC2 build. Verified with WTP 1.0 RC4 driver. This is part of a mass update to close out all stale WTP defects already verified by the reporter but awaiting closure by the assignee. If you feel this defect was closed inappropriately, please reopen. Thanks, John Lanuti |
When I edit a html empty model using DOM API and IDocument API by turns, I got an ArrayIndexOutOfBoundsException. Here is a sample code to recreate the problem: IDOMModel model; // assumes 0-byte html empty file IDOMDocument doc = model.getDocument(); Element ele = doc.createElement(HTML40Namespace.ElementName.P); doc.appendChild(ele); model.getStructuredDocument().set(""); Element ele2 = doc.createElement(HTML40Namespace.ElementName.P); doc.appendChild(ele2); My quick investication indicates html model goes wrong state after IDocument::set(String ) call. After the API call, IStructuredDocument API says the document is empty file, while DOM API says there is a Text node in the document. Then an exception is thrown at the next editing API call. Here is the stack trace: java.lang.ArrayIndexOutOfBoundsException: Array index out of range: 0 at org.eclipse.jface.text.GapTextStore.moveAndResizeGap(GapTextStore.java:109) at org.eclipse.jface.text.GapTextStore.adjustGap(GapTextStore.java:69) at org.eclipse.jface.text.GapTextStore.replace(GapTextStore.java:195) at org.eclipse.wst.sse.core.internal.text.StructuredDocumentTextStore.replace(StructuredDocumentTextStore.java:168) at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.updateDocumentData(BasicStructuredDocument.java:2652) at org.eclipse.wst.sse.core.internal.text.StructuredDocumentReParser.reparse(StructuredDocumentReParser.java:1350) at org.eclipse.wst.sse.core.internal.text.StructuredDocumentReParser.reparse(StructuredDocumentReParser.java:1284) at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.updateModel(BasicStructuredDocument.java:2674) at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.internalReplaceText(BasicStructuredDocument.java:1899) at org.eclipse.wst.sse.core.internal.text.BasicStructuredDocument.replaceText(BasicStructuredDocument.java:2367) at org.eclipse.wst.sse.core.internal.text.JobSafeStructuredDocument.replaceText(JobSafeStructuredDocument.java:86) at org.eclipse.wst.xml.core.internal.document.XMLModelUpdater.replaceSource(XMLModelUpdater.java:1608) at org.eclipse.wst.xml.core.internal.document.XMLModelUpdater.replaceChild(XMLModelUpdater.java:1566) at org.eclipse.wst.xml.core.internal.document.DOMModelImpl.childReplaced(DOMModelImpl.java:187) at org.eclipse.wst.xml.core.internal.document.NodeContainer.notifyChildReplaced(NodeContainer.java:357) at org.eclipse.wst.xml.core.internal.document.NodeContainer.insertBefore(NodeContainer.java:298) at org.eclipse.wst.xml.core.internal.document.NodeContainer.appendChild(NodeContainer.java:129) [...]