On Tue, 2005-12-20 at 11:01 -0500, Anthony Balkissoon wrote:
> On Mon, 2005-12-19 at 17:54 -0500, Anthony Balkissoon wrote:
> > This is the first part of the work I'm doing on DefaultStyledDocument.
> > This fixes a couple issues associated with PR 24744 but the bug is not
> > completely fixed yet, so the testcase for that PR still crashes. I will
> > continue to work on this but these changes are ready to go in and may
> > help get some text/html test apps up and running so I'm committing this
> > now.
>
> This patch caused some Mauve regressions because it's causing an
> unexpected exception, I'm working on this and will also be checking in
> the Mauve tests I wrote while creating this patch.
>
> --Tony
Okay, I've fixed the regressions and made progress with the attached
patch. Yesterday I put code in ElementBuffer.insertContentTag to join
Elements if they had the same attribute sets. This was incorrect and
led to the Mauve regressions. The problem was that the wrong
information was being passed to insertContentTag.
DefaultStyledDocument.insertUpdate basically makes a list of
ElementSpecs that it wants the ElementBuffer to turn into Elements.
There was a problem in this method that was causing the wrong
ElementSpecs to be sent to the ElementBuffer (and eventually to
insertContentTag). This is fixed, but the solution is somewhat of a
hack.
The reason for the hack is as follows. To create a LeafElement (a
structural object) we call AbstractDocument.createLeafElement which
takes as one of its parameters an AttributeSet that you want the
LeafElement to have. However, in the process of creating the
LeafElement we actually add an attribute to the set that describes the
LeafElement's start and end offset positions. So for instance, the
following code would print "false" (the parent, start_offset, end_offset
arguments are not important to this discussion):
SimpleAttributeSet set = new SimpleAttributeSet();
set.addAttribute("A", "B");
Element e = createLeafElement(parent, set, start_offset, end_offset);
System.out.println (e.isEqual(set));
And this is the basis of our problem .. before we were comparing the
AttributeSet that we were going to use to create an Element with the
attribute set from an Element that already existed and so we'd always
get that they were not equal. So the hack basically calls
createLeafElement to create an Element from which we get an
AttributeSet, and we use _that_ set in our comparisons. The element
returned from createLeafElement is then thrown away, we don't actually
add it to the Document structure.
Since I consider this a bit of a hack, comments are appreciated (Roman?)
in case another solution is preferred.
2005-12-20 Anthony Balkissoon <[EMAIL PROTECTED]>
* javax/swing/text/DefaultStyledDocument.java:
(ElementBuffer.insertContentTag): If the direction is
OriginateDirection split all the time, don't check the attribute sets.
Removed the special case for the first insertion. These cases should
fall under the direction JoinPreviousDirection. Changed the comments to
reflect this.
(insertUpdate): Added a hack to get the right result when comparing
the attributes of the new ElementSpec to the attributes of either
the previous or next Element.
--Tony
Index: javax/swing/text/DefaultStyledDocument.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/DefaultStyledDocument.java,v
retrieving revision 1.21
diff -u -r1.21 DefaultStyledDocument.java
--- javax/swing/text/DefaultStyledDocument.java 19 Dec 2005 22:52:32 -0000 1.21
+++ javax/swing/text/DefaultStyledDocument.java 20 Dec 2005 18:24:13 -0000
@@ -866,89 +866,36 @@
removed = new Element[0];
index++;
}
- else if (current.getStartOffset() == offset
- && current.getEndOffset() - 1 == endOffset)
- {
- // This is if the new insertion covers the entire range of
- // <code>current</code>. This will generally happen for the
- // first insertion into a new paragraph.
- added = new Element[1];
- added[0] = createLeafElement(paragraph, tagAtts,
- offset, endOffset + 1);
- }
else if (current.getStartOffset() == offset)
- {
+ {
// This is if the new insertion happens immediately before
- // the <code>current</code> Element. In this case, if there is
- // a split, there are 2 resulting Elements.
-
- AttributeSet splitAtts = splitRes[1].getAttributes();
- if (attributeSetsAreSame(tagAtts, splitAtts))
- {
- // If the attributes of the adjacent Elements are the same
- // then we don't split them, we join them into one.
- added = new Element [1];
- added[0] = createLeafElement(paragraph, tagAtts, offset,
- splitRes[1].getEndOffset());
- }
- else
- {
- // Otherwise we have 2 resulting Elements.
- added = new Element[2];
- added[0] = createLeafElement(paragraph, tagAtts,
- offset, endOffset);
- added[1] = splitRes[1];
- }
+ // the <code>current</code> Element. In this case there are 2
+ // resulting Elements.
+ added = new Element[2];
+ added[0] = createLeafElement(paragraph, tagAtts, offset,
+ endOffset);
+ added[1] = splitRes[1];
}
- else if (current.getEndOffset() == endOffset + 1)
+ else if (current.getEndOffset() == endOffset)
{
// This is if the new insertion happens right at the end of
- // the <code>current</code> Element. In this case, if there is
- // a split, there are 2 resulting Elements.
- AttributeSet splitAtts = splitRes[0].getAttributes();
- if (attributeSetsAreSame(tagAtts, splitAtts))
- {
- // If the attributes are the same, no need to split.
- added = new Element [1];
- added[0] = createLeafElement(paragraph, tagAtts,
- splitRes[0].getStartOffset(),
- endOffset + 1);
- }
- else
- {
- // Otherwise there are 2 resulting Elements.
- added = new Element[2];
- added[0] = splitRes[0];
- added[1] = createLeafElement(paragraph, tagAtts, offset,
- endOffset + 1);
- }
+ // the <code>current</code> Element. In this case there are
+ // 2 resulting Elements.
+ added = new Element[2];
+ added[0] = splitRes[0];
+ added[1] = createLeafElement(paragraph, tagAtts, offset,
+ endOffset);
}
else
{
// This is if the new insertion is in the middle of the
- // <code>current</code> Element. In this case, if there is a
- // split, there will be 3 resulting Elements. Note, since
- // <code>splitRes[0]</code> and <code>splitRes[1]</code> were
- // once the same Element, they have the same attributes.
- AttributeSet split1Atts = splitRes[0].getAttributes();
-
- if (attributeSetsAreSame(tagAtts, split1Atts))
- {
- // If the attributes are the same, no need to split.
- added = new Element [1];
- added[0] = createLeafElement(paragraph, tagAtts,
- splitRes[0].getStartOffset(),
- splitRes[1].getEndOffset());
- }
- else
- {
- // Otherwise there are 3 resulting Elements.
- added = new Element[3];
- added[0] = splitRes[0];
- added[1] = createLeafElement(paragraph, tagAtts, offset,
- endOffset);
- added[2] = splitRes[1];
- }
+ // <code>current</code> Element. In this case
+ // there will be 3 resulting Elements.
+ added = new Element[3];
+ added[0] = splitRes[0];
+ added[1] = createLeafElement(paragraph, tagAtts, offset,
+ endOffset);
+ added[2] = splitRes[1];
}
paragraph.replace(index, removed.length, added);
addEdit(paragraph, index, removed, added);
@@ -1529,10 +1476,11 @@
*/
protected void insertUpdate(DefaultDocumentEvent ev, AttributeSet attr)
{
- super.insertUpdate(ev, attr);
+ super.insertUpdate(ev, attr);
int offset = ev.getOffset();
int length = ev.getLength();
int endOffset = offset + length;
+ Element newElement, newElement2;
AttributeSet paragraphAttributes =
getParagraphElement(endOffset).getAttributes();
Segment txt = new Segment();
@@ -1561,19 +1509,28 @@
{
ElementSpec spec = new ElementSpec(attr, ElementSpec.ContentType,
len);
-
+ // FIXME: This is a hack. Without this, our check to see if
+ // prev.getAttributes was the same as the attribute set for the new
+ // ElementSpec was never true, because prev has an attribute key
+ // LeafElement(content) that contains its start and end offset as
+ // the value. Similarly for next.
+ newElement = createLeafElement(null, attr, prev.getStartOffset(),
+ prev.getEndOffset());
+ newElement2 = createLeafElement(null, attr, next.getStartOffset(),
+ next.getEndOffset());
+
// If we are at the last index, then check if we could probably be
// joined with the next element.
if (i == segmentEnd - 1)
{
- if (next.getAttributes().isEqual(attr))
+ if (next.getAttributes().isEqual(newElement2.getAttributes()))
spec.setDirection(ElementSpec.JoinNextDirection);
}
// If we are at the first new element, then check if it could be
// joined with the previous element.
else if (specs.size() == 0)
{
- if (prev.getAttributes().isEqual(attr))
+ if (prev.getAttributes().isEqual(newElement.getAttributes()))
spec.setDirection(ElementSpec.JoinPreviousDirection);
}
@@ -1595,15 +1552,26 @@
if (len > 0)
{
ElementSpec spec = new ElementSpec(attr, ElementSpec.ContentType, len);
+
+ // FIXME: This is a hack. Without this, our check to see if
+ // prev.getAttributes was the same as the attribute set for the new
+ // ElementSpec was never true, because prev has an attribute key
+ // LeafElement(content) that contains its start and end offset as
+ // the value. Similarly for next.
+ newElement = createLeafElement(null, attr, prev.getStartOffset(),
+ prev.getEndOffset());
+ newElement2 = createLeafElement(null, attr, next.getStartOffset(),
+ next.getEndOffset());
+
// If we are at the first new element, then check if it could be
// joined with the previous element.
if (specs.size() == 0)
{
- if (prev.getAttributes().isEqual(attr))
+ if (prev.getAttributes().isEqual(newElement.getAttributes()))
spec.setDirection(ElementSpec.JoinPreviousDirection);
}
// Check if we could probably be joined with the next element.
- else if (next.getAttributes().isEqual(attr))
+ else if (next.getAttributes().isEqual(newElement2.getAttributes()))
spec.setDirection(ElementSpec.JoinNextDirection);
specs.add(spec);
_______________________________________________
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
