Mozilla's DOM Ranges don't support comment nodes

RESOLVED FIXED in Future

Status

()

defect
RESOLVED FIXED
17 years ago
Last year

People

(Reporter: aaronlev, Assigned: mozeditor)

Tracking

({qawanted})

Trunk
Future
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

17 years ago
If one tries to use nsIDOMRange::SetStart() or SetEnd() with a comment node, it
returns an error.

This is because nsRange::GetNodeLength() does not handle comment nodes.
It only supports node length for text nodes and cdata section nodes.

However, I noticed there are other places in dom ranges where comment nodes are
not supported. Is this by design or was it just never completed?
Reporter

Comment 1

17 years ago
Is this the right thing to do? Seeking r=/sr=
Reporter

Updated

17 years ago
Blocks: 166340
Reporter

Comment 2

17 years ago
I already have a work around for my particular bug, so I'm going to pass this to
jrfrancis, so he can get all the places that should support comment nodes right.

On the other hand, if this doesn't seem to affect anyone it doesn't bother me if
it's futured.
Assignee: aaronl → jfrancis
Summary: nsIDOMRange::GetNodeLength() returning incorrect value for comment nodes → Mozilla's DOM Ranges don't support comment nodes
What does the DOM Range spec say on this?  This sounds to me like something we
should really fix....
Reporter

Updated

17 years ago
Blocks: 175046
Assignee

Comment 4

17 years ago
futuring for now.  if someone is affected by this please comment.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
I think my fix for bug 332148 handles this, if it's not fixed already.
Depends on: 332148
Someone want to submit a testcase beyond the one provided in bug 332148?
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Alex, does that testcase test comment nodes?  If not, care to just write a test for this bug?
bz: it does, but only for the extractContents and cloneContents cases.  IMHO, that's not enough.  :)

I'd really rather not write the tests myself (nor would I want to burden you, bz).
So setStart/setEnd per comment 0 work?  That's what needs to be tested.

For what it's worth, this is now in my to-do list, since _someone_ has to do it.  I'd be quite happy if someone else gets to it, of course.
Keywords: qawanted
(In reply to comment #9)
> So setStart/setEnd per comment 0 work?  That's what needs to be tested.

Yes, that's one thing the tests for extractContents covers.
Sounds like this bug is covered then... what else needs testing?
Component: DOM: Traversal-Range → DOM: Core & HTML
Depends on: 1166383
No longer depends on: 1166383
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.