You are viewing a plain text version of this content. The canonical link for it is here.
Posted to j-dev@xerces.apache.org by "Ludger Bünger (JIRA)" <xe...@xml.apache.org> on 2009/12/14 16:30:18 UTC

[jira] Updated: (XERCESJ-1400) Range implementation: several bugs (see detailed Description) and code Maintenance

     [ https://issues.apache.org/jira/browse/XERCESJ-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ludger Bünger updated XERCESJ-1400:
-----------------------------------

    Description: 
Ok, there are a couple of issues (bugs, non-standard-compliant behaviour and maintenance) in Xerces Range implementation I created fixes for which I like to provide to the community.

These Fixes are already in use in our  application depending heavily upon Ranges and have been deployed successfully at customers since more than  two years.



Bug 1:
Quite a lot of Range methods are not aware of character type Nodes other than Text. 

Described more in detail in  [1], the Ranges spec differentiate between two type of Nodes.
a) Nodes of type CharacterData (these include Text), Comment or ProcessingInstruction which are treated the same in respect to the Ranges spec.
b) other Nodes.

The inital implementation of Ranges only considered the case of Text nodes ignoring the other character containing node types.
With the Fixes for Bug #1228 and #1229 this has been improved for specific cases but not in general.

The following methods of RangeImpl are not aware of all above mentioned character containing nodes:

* RangeImpl.insertNode
* RangeImpl.surroundContent
* RangeImpl.selectNodeContent
* RangeImpl.toString
* RangeImpl.receiveSplitData
* RangeImpl.traverseNode
* RangeImpl.getSelectedNode

The following methods are correctly aware of them and have already been fixed by Michael:

* RangeImpl.checkindex
* RangeImpl.receiveDeletedText
* RangeImpl.receiveInsertedText
* RangeImpl.receiveSplitData
* RangeImpl.traverseSameContainer


Bug 2:
selectNodeContents does not surround character containing nodes.

consider following code snippet.

Text text = doc.createNode();
text.setData("String");  // length of string: 6
range.selectNodeContent(text);

received result: (text, 0, text, 0);
correct result: (text, 0, text, 6);


Bug 3:
surroundContent(newParent) currently does not remove children of newParent as specified in [2]:

"If the node newParent has any children, those children are removed before its insertion. Also, if the node newParent already has a parent, it is removed from the original parent's childNodes list."

Bug 4:
Text.textSplit does not update Ranges at all.

Solution: notify document using document.splitData after text split to cause range update.

Bug 5:
After calling insertNode Ranges containing the same affected node are not updated correctly.

Suggested Solution: use textSplit instead of doing a manual text split inside RangeImpl and set content of this range afterwards.
This does work now due to Bug 4 being fixed.

However textSplit is only defined upon Nodes of type Text, while ranges might split any character data containing nodes.
Thus I suggest to extract the text split operation to an internal method that is implemented upon CharacterDataImpl used by both Text.textSplit and Range.insertNode.


Maintenance 6:
Obsolete methods.

The following methods are never used and thus obsolete:

* deleteData
* insertData
* removeChild
* signalSplitData

Maintenance 7:
Obsolete fields.

After removing methods from Maintenance issue 6, the following fields are never written and thus obsolete:

* fDeleteNode
* fInsertedFromRange
* fRemoveChild
* fSplitNode

I suggest removing these.
Range update works perfectly well without.

[1] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Position
[2] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Surrounding


  was:
Ok, there are a couple of issues (bugs, non-standard-compliant behaviour and maintenance) in Xerces Range implementation I created fixes for which I like to provide to the community.

These Fixes are already in use in our  application depending heavily upon Ranges and have been deployed successfully at customers since more than  two years.



Bug 1:
Quite a lot of Range methods are not aware of character type Nodes other than Text. 

Described more in detail in  [1], the Ranges spec differentiate between two type of Nodes.
a) Nodes of type CharacterData (these include Text), Comment or ProcessingInstruction which are treated the same in respect to the Ranges spec.
b) other Nodes.

The inital implementation of Ranges only considered the case of Text nodes ignoring the other character containing node types.
With the Fixes for Bug #1228 and #1229 this has been improved for specific cases but not in general.

The following methods of RangeImpl are not aware of all above mentioned character containing nodes:

* RangeImpl.insertNode
* RangeImpl.surroundContent
* RangeImpl.selectNodeContent
* RangeImpl.toString
* RangeImpl.receiveSplitData
* RangeImpl.traverseNode
* RangeImpl.getSelectedNode

The following methods are correctly aware of them and have already been fixed by Michael:

* RangeImpl.checkindex
* RangeImpl.receiveDeletedText
* RangeImpl.receiveInsertedText
* RangeImpl.receiveSplitData
* RangeImpl.traverseSameContainer


Bug 2:
selectNodeContents does not surround character containing nodes.

consider following code snippet.

Text text = doc.createNode();
text.setData("String");  // length of string: 6
range.selectNodeContent(text);

received result: (text, 0, text, 0);
correct result: (text, 0, text, 6);


Bug 3:
surroundContent(newParent) does remove children of newParent as sepcified in [2].

"If the node newParent has any children, those children are removed before its insertion. Also, if the node newParent already has a parent, it is removed from the original parent's childNodes list."

Bug 4:
Text.textSplit does not update Ranges at all.

Solution: notify document using document.splitData after text split to cause range update.

Bug 5:
After calling insertNode Ranges containing the same affected node are not updated correctly.

Suggested Solution: use textSplit instead of doing a manual text split inside RangeImpl and set content of this range afterwards.
This does work now due to Bug 4 being fixed.

However textSplit is only defined upon Nodes of type Text, while ranges might split any character data containing nodes.
Thus I suggest to extract the text split operation to an internal method that is implemented upon CharacterDataImpl used by both Text.textSplit and Range.insertNode.


Maintenance 6:
Obsolete methods.

The following methods are never used and thus obsolete:

* deleteData
* insertData
* removeChild
* signalSplitData

Maintenance 7:
Obsolete fields.

After removing methods from Maintenance issue 6, the following fields are never written and thus obsolete:

* fDeleteNode
* fInsertedFromRange
* fRemoveChild
* fSplitNode

I suggest removing.
Range update works perfectly well without these.

[1] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Position
[2] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Surrounding



Added missing NOT in description of Bug 3 (description did not make sense without).

> Range implementation: several bugs (see detailed Description) and code Maintenance
> ----------------------------------------------------------------------------------
>
>                 Key: XERCESJ-1400
>                 URL: https://issues.apache.org/jira/browse/XERCESJ-1400
>             Project: Xerces2-J
>          Issue Type: Bug
>          Components: DOM (Level 2 Traversal & Range)
>    Affects Versions: 2.9.1
>            Reporter: Ludger Bünger
>         Attachments: Xerces-Range-Maintenance-Patch.txt
>
>
> Ok, there are a couple of issues (bugs, non-standard-compliant behaviour and maintenance) in Xerces Range implementation I created fixes for which I like to provide to the community.
> These Fixes are already in use in our  application depending heavily upon Ranges and have been deployed successfully at customers since more than  two years.
> Bug 1:
> Quite a lot of Range methods are not aware of character type Nodes other than Text. 
> Described more in detail in  [1], the Ranges spec differentiate between two type of Nodes.
> a) Nodes of type CharacterData (these include Text), Comment or ProcessingInstruction which are treated the same in respect to the Ranges spec.
> b) other Nodes.
> The inital implementation of Ranges only considered the case of Text nodes ignoring the other character containing node types.
> With the Fixes for Bug #1228 and #1229 this has been improved for specific cases but not in general.
> The following methods of RangeImpl are not aware of all above mentioned character containing nodes:
> * RangeImpl.insertNode
> * RangeImpl.surroundContent
> * RangeImpl.selectNodeContent
> * RangeImpl.toString
> * RangeImpl.receiveSplitData
> * RangeImpl.traverseNode
> * RangeImpl.getSelectedNode
> The following methods are correctly aware of them and have already been fixed by Michael:
> * RangeImpl.checkindex
> * RangeImpl.receiveDeletedText
> * RangeImpl.receiveInsertedText
> * RangeImpl.receiveSplitData
> * RangeImpl.traverseSameContainer
> Bug 2:
> selectNodeContents does not surround character containing nodes.
> consider following code snippet.
> Text text = doc.createNode();
> text.setData("String");  // length of string: 6
> range.selectNodeContent(text);
> received result: (text, 0, text, 0);
> correct result: (text, 0, text, 6);
> Bug 3:
> surroundContent(newParent) currently does not remove children of newParent as specified in [2]:
> "If the node newParent has any children, those children are removed before its insertion. Also, if the node newParent already has a parent, it is removed from the original parent's childNodes list."
> Bug 4:
> Text.textSplit does not update Ranges at all.
> Solution: notify document using document.splitData after text split to cause range update.
> Bug 5:
> After calling insertNode Ranges containing the same affected node are not updated correctly.
> Suggested Solution: use textSplit instead of doing a manual text split inside RangeImpl and set content of this range afterwards.
> This does work now due to Bug 4 being fixed.
> However textSplit is only defined upon Nodes of type Text, while ranges might split any character data containing nodes.
> Thus I suggest to extract the text split operation to an internal method that is implemented upon CharacterDataImpl used by both Text.textSplit and Range.insertNode.
> Maintenance 6:
> Obsolete methods.
> The following methods are never used and thus obsolete:
> * deleteData
> * insertData
> * removeChild
> * signalSplitData
> Maintenance 7:
> Obsolete fields.
> After removing methods from Maintenance issue 6, the following fields are never written and thus obsolete:
> * fDeleteNode
> * fInsertedFromRange
> * fRemoveChild
> * fSplitNode
> I suggest removing these.
> Range update works perfectly well without.
> [1] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Position
> [2] http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html#Level-2-Range-Surrounding

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org