Range.extractContents doesn't clone partially selected nodes

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: dbaron, Assigned: smaug)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment)

http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Extracting says that Range.extractContents should clone nodes that are partially selected by the range.

This is tested by test 9 of Acid3, which calls extractContents on a range.  The test there is actually reasonably clear; it expects the markup:
      // <h1><em>ful<\em> Kitty<\h1><p><\p>
as shown, but we return a fragment containing just two text nodes, "ful" and " Kitty".
I should pay attention to this too - I wrote most of the original patch which smaug drove to completion.  So this is as much my bustage as his.  :(
Blocks: 332148
Alex, if you have a patch ready, feel free to take this, but I started to
look at this yesterday.
Assignee: nobody → Olli.Pettay
It's all yours, smaug - I don't have that fast turnaround time right now.  I'm thinking the automated tests will change quite a bit from this bug.
Posted patch wip1Splinter Review
This is a bit tricky. Based partly on CloneContents code, but because iteration 
is done backwards, the code is a bit different. And Extract/Delete don't of 
course clone usually so that brings in some complexity. 
The actual code to handle cloning parent nodes isn't that long, 
~55 lines in CutContents. Other changes are either simplifying the code
or adding support for the case when element is selected but nothing in it is.

Eventually CloneContents and CutContents should merge, but maybe not in this bug. I'd actually like to do that in a followup, since that merge will be
based on CutContents code, not CloneContents.

With bug 302775 this fixes the last range bug in ACID3.
Attachment #339094 - Flags: review?(jonas)
For the benefit of those of you playing along at home, you need to apply this patch first, then apply the one from bug 302775 specifying -F6 on the patch command.
Well don't know if it is related to this bug or bug 302775, but I cannot get firefox to be built.

It crashes when building nsDOMAttribute.cpp.

/home/fred/logs/fx2/src/content/base/src/nsDOMAttribute.cpp:59:24: erreur: nsTextNode.h : Aucun fichier ou dossier de ce type

=> No such file or directory.

And it stops building :(
nsTextNode.h is added and used in bug 302775. Should have nothing to do with this bug.
Jonas, any chance for a review.
This is something we really should get into 1.9.1, even if this wasn't
an ACID3 bug.
Flags: blocking1.9.1?
Attachment #339094 - Flags: superreview+
Attachment #339094 - Flags: review?(jonas)
Attachment #339094 - Flags: review+
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: blocking1.9.1?
Resolution: --- → FIXED
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.