In an effort to clear the Mauve regressions for Swing I looked into
AbstractDocument and GapContent again. Basically I removed some explicit
checks to not throw BadLocationException in strange cases. This is
compliant with the RI, but bends the spec a little. See that other
thread on [EMAIL PROTECTED]
A while ago I started the bidi stuff in AbstractDocument. This will now
have to go in here, as I don't want to throw it out again. It's not
complete yet.
This patch should make some Mauve and Intel tests happy.
2006-07-27 Roman Kennke <[EMAIL PROTECTED]>
* javax/swing/text/AbstractDocument.java
(documentCV): Made field private.
(bypass): Made field private.
(bidiRoot): New field.
(AbstractDocument): Initialize bidiRoot.
(getBidiRootElement): Return bidiRoot.
(getRootElements): Adjusted to also return the bidiRoot element.
(BranchElement.startOffset): Removed unneeded field.
(BranchElement.endOffset): Removed unneeded field.
(BranchElement.BranchElement): Removed unneeded fields.
(BranchElement.getEndOffset): Don't explicitly throw NPE here. This is
done automatically when there's no element left in the array.
(BranchElement.getStartOffset): Likewise.
(BranchElement.replace): Reordered calculations to avoid double
calculations.
(removeImpl): Silently ignore requests with length <= 0.
* javax/swing/text/GapContent.java
(createPosition): Removed explicit check for correct offset.
This class can deal with offsets outside the document.
(shiftEnd): Update all positions, even those outside the
document.
(adjustPositionsInRange): Fixed to also adjust positions outside
the document boundary.
/Roman
Index: javax/swing/text/AbstractDocument.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/AbstractDocument.java,v
retrieving revision 1.59
diff -u -1 -2 -r1.59 AbstractDocument.java
--- javax/swing/text/AbstractDocument.java 22 Jun 2006 15:14:00 -0000 1.59
+++ javax/swing/text/AbstractDocument.java 27 Jul 2006 15:15:25 -0000
@@ -138,32 +138,37 @@
* The number of readers. Used for locking.
*/
private int numReaders = 0;
/**
* Tells if there are one or more writers waiting.
*/
private int numWritersWaiting = 0;
/**
* A condition variable that readers and writers wait on.
*/
- Object documentCV = new Object();
+ private Object documentCV = new Object();
/** An instance of a DocumentFilter.FilterBypass which allows calling
* the insert, remove and replace method without checking for an installed
* document filter.
*/
- DocumentFilter.FilterBypass bypass;
-
+ private DocumentFilter.FilterBypass bypass;
+
+ /**
+ * The bidi root element.
+ */
+ private Element bidiRoot;
+
/**
* Creates a new <code>AbstractDocument</code> with the specified
* [EMAIL PROTECTED] Content} model.
*
* @param doc the <code>Content</code> model to be used in this
* <code>Document<code>
*
* @see GapContent
* @see StringContent
*/
protected AbstractDocument(Content doc)
{
@@ -180,24 +185,27 @@
*
* @see GapContent
* @see StringContent
*/
protected AbstractDocument(Content doc, AttributeContext ctx)
{
content = doc;
context = ctx;
// FIXME: This is determined using a Mauve test. Make the document
// actually use this.
putProperty("i18n", Boolean.FALSE);
+
+ // FIXME: Fully implement bidi.
+ bidiRoot = new BranchElement(null, null);
}
/** Returns the DocumentFilter.FilterBypass instance for this
* document and create it if it does not exist yet.
*
* @return This document's DocumentFilter.FilterBypass instance.
*/
private DocumentFilter.FilterBypass getBypass()
{
if (bypass == null)
bypass = new Bypass();
@@ -355,25 +363,25 @@
protected final AttributeContext getAttributeContext()
{
return context;
}
/**
* Returns the root element for bidirectional content.
*
* @return the root element for bidirectional content
*/
public Element getBidiRootElement()
{
- return null;
+ return bidiRoot;
}
/**
* Returns the [EMAIL PROTECTED] Content} model for this <code>Document</code>
*
* @return the [EMAIL PROTECTED] Content} model for this <code>Document</code>
*
* @see GapContent
* @see StringContent
*/
protected final Content getContent()
{
@@ -472,26 +480,27 @@
/**
* Returns all root elements of this <code>Document</code>. By default
* this just returns the single root element returned by
* [EMAIL PROTECTED] #getDefaultRootElement()}. <code>Document</code> implementations
* that support multiple roots must override this method and return all roots
* here.
*
* @return all root elements of this <code>Document</code>
*/
public Element[] getRootElements()
{
- Element[] elements = new Element[1];
+ Element[] elements = new Element[2];
elements[0] = getDefaultRootElement();
+ elements[1] = getBidiRootElement();
return elements;
}
/**
* Returns a [EMAIL PROTECTED] Position} which will always mark the beginning of the
* <code>Document</code>.
*
* @return a [EMAIL PROTECTED] Position} which will always mark the beginning of the
* <code>Document</code>
*/
public final Position getStartPosition()
{
@@ -734,53 +743,54 @@
* document
*/
public void remove(int offset, int length) throws BadLocationException
{
if (documentFilter == null)
removeImpl(offset, length);
else
documentFilter.remove(getBypass(), offset, length);
}
void removeImpl(int offset, int length) throws BadLocationException
{
- if (offset < 0 || offset > getLength())
- throw new BadLocationException("Invalid remove position", offset);
-
- if (offset + length > getLength())
- throw new BadLocationException("Invalid remove length", offset);
+ // The RI silently ignores all requests that have a negative length.
+ // Don't ask my why, but that's how it is.
+ if (length > 0)
+ {
+ if (offset < 0 || offset > getLength())
+ throw new BadLocationException("Invalid remove position", offset);
- // Prevent some unneccessary method invocation (observed in the RI).
- if (length == 0)
- return;
+ if (offset + length > getLength())
+ throw new BadLocationException("Invalid remove length", offset);
- DefaultDocumentEvent event =
- new DefaultDocumentEvent(offset, length,
- DocumentEvent.EventType.REMOVE);
+ DefaultDocumentEvent event =
+ new DefaultDocumentEvent(offset, length,
+ DocumentEvent.EventType.REMOVE);
- try
- {
- writeLock();
+ try
+ {
+ writeLock();
- // The order of the operations below is critical!
- removeUpdate(event);
- UndoableEdit temp = content.remove(offset, length);
+ // The order of the operations below is critical!
+ removeUpdate(event);
+ UndoableEdit temp = content.remove(offset, length);
- postRemoveUpdate(event);
- fireRemoveUpdate(event);
+ postRemoveUpdate(event);
+ fireRemoveUpdate(event);
+ }
+ finally
+ {
+ writeUnlock();
+ }
}
- finally
- {
- writeUnlock();
- }
}
/**
* Replaces a piece of content in this <code>Document</code> with
* another piece of content.
*
* <p>If a [EMAIL PROTECTED] DocumentFilter} is installed in this document, the
* corresponding method of the filter object is called.</p>
*
* <p>The method has no effect if <code>length</code> is zero (and
* only zero) and, at the same time, <code>text</code> is
* <code>null</code> or has zero length.</p>
@@ -1686,50 +1696,36 @@
/**
* The child elements of this BranchElement.
*/
private Element[] children;;
/**
* The number of children in the branch element.
*/
private int numChildren;
/**
- * The cached startOffset value. This is used in the case when a
- * BranchElement (temporarily) has no child elements.
- */
- private int startOffset;
-
- /**
- * The cached endOffset value. This is used in the case when a
- * BranchElement (temporarily) has no child elements.
- */
- private int endOffset;
-
- /**
* Creates a new <code>BranchElement</code> with the specified
* parent and attributes.
*
* @param parent the parent element of this <code>BranchElement</code>
* @param attributes the attributes to set on this
* <code>BranchElement</code>
*/
public BranchElement(Element parent, AttributeSet attributes)
{
super(parent, attributes);
children = new Element[1];
numChildren = 0;
- startOffset = -1;
- endOffset = -1;
}
/**
* Returns the children of this <code>BranchElement</code>.
*
* @return the children of this <code>BranchElement</code>
*/
public Enumeration children()
{
if (children.length == 0)
return null;
@@ -1824,33 +1820,29 @@
* Returns the offset inside the document model that is after the last
* character of this element.
* This is the end offset of the last child element. If this element
* has no children, this method throws a <code>NullPointerException</code>.
*
* @return the offset inside the document model that is after the last
* character of this element
*
* @throws NullPointerException if this branch element has no children
*/
public int getEndOffset()
{
- if (numChildren == 0)
- {
- if (endOffset == -1)
- throw new NullPointerException("BranchElement has no children.");
- }
- else
- endOffset = children[numChildren - 1].getEndOffset();
-
- return endOffset;
+ // This might accss one cached element or trigger an NPE for
+ // numChildren == 0. This is checked by a Mauve test.
+ Element child = numChildren > 0 ? children[numChildren - 1]
+ : children[0];
+ return child.getEndOffset();
}
/**
* Returns the name of this element. This is [EMAIL PROTECTED] #ParagraphElementName}
* in this case.
*
* @return the name of this element
*/
public String getName()
{
return ParagraphElementName;
}
@@ -1858,33 +1850,31 @@
/**
* Returns the start offset of this element inside the document model.
* This is the start offset of the first child element. If this element
* has no children, this method throws a <code>NullPointerException</code>.
*
* @return the start offset of this element inside the document model
*
* @throws NullPointerException if this branch element has no children and
* no startOffset value has been cached
*/
public int getStartOffset()
{
- if (numChildren == 0)
- {
- if (startOffset == -1)
- throw new NullPointerException("BranchElement has no children.");
- }
- else
- startOffset = children[0].getStartOffset();
-
- return startOffset;
+ // Do not explicitly throw an NPE here. If the first element is null
+ // then the NPE gets thrown anyway. If it isn't, then it either
+ // holds a real value (for numChildren > 0) or a cached value
+ // (for numChildren == 0) as we don't fully remove elements in replace()
+ // when removing single elements.
+ // This is checked by a Mauve test.
+ return children[0].getStartOffset();
}
/**
* Returns <code>false</code> since <code>BranchElement</code> are no
* leafes.
*
* @return <code>false</code> since <code>BranchElement</code> are no
* leafes
*/
public boolean isLeaf()
{
return false;
@@ -1915,45 +1905,44 @@
return null;
}
/**
* Replaces a set of child elements with a new set of child elemens.
*
* @param offset the start index of the elements to be removed
* @param length the number of elements to be removed
* @param elements the new elements to be inserted
*/
public void replace(int offset, int length, Element[] elements)
{
- if (numChildren + elements.length - length > children.length)
+ int delta = elements.length - length;
+ int copyFrom = offset + length; // From where to copy.
+ int copyTo = copyFrom + delta; // Where to copy to.
+ int numMove = numChildren - copyFrom; // How many elements are moved.
+ if (numChildren + delta > children.length)
{
// Gotta grow the array.
- int newSize = Math.max(2 * children.length,
- numChildren + elements.length - length);
+ int newSize = Math.max(2 * children.length, numChildren + delta);
Element[] target = new Element[newSize];
System.arraycopy(children, 0, target, 0, offset);
System.arraycopy(elements, 0, target, offset, elements.length);
- System.arraycopy(children, offset + length, target,
- offset + elements.length,
- numChildren - offset - length);
+ System.arraycopy(children, copyFrom, target, copyTo, numMove);
children = target;
}
else
{
- System.arraycopy(children, offset + length, children,
- offset + elements.length,
- numChildren - offset - length);
+ System.arraycopy(children, copyFrom, children, copyTo, numMove);
System.arraycopy(elements, 0, children, offset, elements.length);
}
- numChildren += elements.length - length;
+ numChildren += delta;
}
/**
* Returns a string representation of this element.
*
* @return a string representation of this element
*/
public String toString()
{
return ("BranchElement(" + getName() + ") "
+ getStartOffset() + "," + getEndOffset() + "\n");
}
Index: javax/swing/text/GapContent.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/GapContent.java,v
retrieving revision 1.53
diff -u -1 -2 -r1.53 GapContent.java
--- javax/swing/text/GapContent.java 26 Jul 2006 07:55:19 -0000 1.53
+++ javax/swing/text/GapContent.java 27 Jul 2006 15:15:25 -0000
@@ -523,26 +523,28 @@
/**
* Creates and returns a mark at the specified position.
*
* @param offset the position at which to create the mark
*
* @return the create Position object for the mark
*
* @throws BadLocationException if the offset is not a valid position in the
* buffer
*/
public Position createPosition(final int offset) throws BadLocationException
{
- if (offset < 0 || offset > length())
- throw new BadLocationException("Position offset out of bounds", offset);
+ // Implementation note: We used to perform explicit check on the offset
+ // here. However, this makes some Mauve and Intel/Harmony tests fail
+ // and luckily enough the GapContent can very well deal with offsets
+ // outside the buffer bounds. So I removed that check.
// We try to find a GapContentPosition at the specified offset and return
// that. Otherwise we must create a new one.
GapContentPosition pos = null;
Set positionSet = positions.keySet();
for (Iterator i = positionSet.iterator(); i.hasNext();)
{
GapContentPosition p = (GapContentPosition) i.next();
if (p.getOffset() == offset)
{
pos = p;
break;
@@ -565,25 +567,25 @@
* of the new buffer array. This does change the gapEnd mark but not the
* gapStart mark.
*
* @param newSize the new size of the gap
*/
protected void shiftEnd(int newSize)
{
assert newSize > (gapEnd - gapStart) : "The new gap size must be greater "
+ "than the old gap size";
int delta = newSize - gapEnd + gapStart;
// Update the marks after the gapEnd.
- adjustPositionsInRange(gapEnd, buffer.length, delta);
+ adjustPositionsInRange(gapEnd, -1, delta);
// Copy the data around.
char[] newBuf = (char[]) allocateArray(length() + newSize);
System.arraycopy(buffer, 0, newBuf, 0, gapStart);
System.arraycopy(buffer, gapEnd, newBuf, gapStart + newSize, buffer.length
- gapEnd);
gapEnd = gapStart + newSize;
buffer = newBuf;
}
/**
@@ -780,43 +782,49 @@
for (int i = startIndex; i <= endIndex; i++)
((Mark) marks.get(i)).mark = toStart ? start : end;
}
}
/**
* Adjusts the mark of all <code>Position</code>s that are in the range
* specified by <code>offset</code> and </code>length</code> within
* the buffer array by <code>increment</code>
*
* @param startOffs the start offset of the range to search
- * @param endOffs the length of the range to search
+ * @param endOffs the length of the range to search, -1 means all to the end
* @param incr the increment
*/
private void adjustPositionsInRange(int startOffs, int endOffs, int incr)
{
synchronized (this)
{
// Find the start and end indices in the positionMarks array.
Mark m = new Mark(0); // For comparison / search only.
m.mark = startOffs;
int startIndex = search(marks, m);
if (startIndex < 0) // Translate to insertion index, if not found.
startIndex = - startIndex - 1;
m.mark = endOffs;
- int endIndex = search(marks, m);
- if (endIndex < 0) // Translate to insertion index - 1, if not found.
- endIndex = - endIndex - 2;
+ int endIndex;
+ if (endOffs == -1)
+ endIndex = marks.size() - 1;
+ else
+ {
+ endIndex = search(marks, m);
+ if (endIndex < 0) // Translate to insertion index - 1, if not found.
+ endIndex = - endIndex - 2;
+ }
// Actually adjust the marks.
for (int i = startIndex; i <= endIndex; i++) {
((Mark) marks.get(i)).mark += incr;
}
}
}
/**
* Resets all <code>Position</code> that have an offset of <code>0</code>,
* to also have an array index of <code>0</code>. This might be necessary
* after a call to <code>shiftGap(0)</code>, since then the marks at offset
