Closed
Bug 454325
Opened 16 years ago
Closed 16 years ago
Range.extractContents doesn't clone partially selected nodes
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: smaug)
References
()
Details
Attachments
(1 file)
23.90 KB,
patch
|
sicking
:
review+
sicking
:
superreview+
|
Details | Diff | Splinter Review |
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".
Comment 1•16 years ago
|
||
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
Assignee | ||
Comment 2•16 years ago
|
||
Alex, if you have a patch ready, feel free to take this, but I started to look at this yesterday.
Assignee: nobody → Olli.Pettay
Comment 3•16 years ago
|
||
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.
Assignee | ||
Comment 4•16 years ago
|
||
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)
Comment 5•16 years ago
|
||
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.
Comment 6•16 years ago
|
||
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 :(
Assignee | ||
Comment 7•16 years ago
|
||
nsTextNode.h is added and used in bug 302775. Should have nothing to do with this bug.
Assignee | ||
Comment 8•16 years ago
|
||
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+
Assignee | ||
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: blocking1.9.1?
Resolution: --- → FIXED
Updated•11 years ago
|
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•