Bug 123704

Summary: Switch createContextualFragment to element iterator
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
better patch kling: review+

Description Antti Koivisto 2013-11-03 04:26:56 PST
more iterator
Comment 1 Antti Koivisto 2013-11-03 04:30:56 PST
Created attachment 215865 [details]
patch
Comment 2 Antti Koivisto 2013-11-03 06:43:28 PST
Created attachment 215866 [details]
better patch
Comment 3 Andreas Kling 2013-11-03 12:14:53 PST
Comment on attachment 215866 [details]
better patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215866&action=review

r=me

> Source/WebCore/editing/FrameSelection.cpp:1967
> +            if (!contentDocument)
> +                continue;

This looks new. Sneaky.
Comment 4 Antti Koivisto 2013-11-03 12:18:25 PST
https://trac.webkit.org/r158537
Comment 5 Chris Dumez 2014-11-03 21:52:50 PST
Comment on attachment 215866 [details]
better patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215866&action=review

> Source/WebCore/editing/markup.cpp:913
> +            collectElementsToRemoveFromFragment(element);

This statement has no effect as the result value is being ignored.
Comment 6 Antti Koivisto 2014-11-04 07:17:02 PST
Comment on attachment 215866 [details]
better patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215866&action=review

>> Source/WebCore/editing/markup.cpp:913
>> +            collectElementsToRemoveFromFragment(element);
> 
> This statement has no effect as the result value is being ignored.

Good catch!
Comment 7 Chris Dumez 2014-11-04 08:42:40 PST
(In reply to comment #6)
> Comment on attachment 215866 [details]
> better patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=215866&action=review
> 
> >> Source/WebCore/editing/markup.cpp:913
> >> +            collectElementsToRemoveFromFragment(element);
> > 
> > This statement has no effect as the result value is being ignored.
> 
> Good catch!

The thing is that I am not sure it is actually possible to have an <html> here. I have been trying to test the problem but I cannot get createFragmentForInnerOuterHTML() to return an <html> as first child in the DocumentFragment, despite the input having an <html> tag:
http://jsfiddle.net/2qevwvs3/8/