Bug 47300

Summary: Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, enrica, justin.garcia, leviw, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
demo
none
fixes the bug tony: review+

Description Ryosuke Niwa 2010-10-06 13:53:41 PDT
Created attachment 69986 [details]
demo

The attached demo causes WebKit TOT to hit the first ASSERT in moveParagraph:
ASSERT(isStartOfParagraph(startOfParagraphToMove));
Comment 1 Ryosuke Niwa 2010-10-28 16:17:03 PDT
We no longer hit the assert but we don't remove pre, which seems to be wrong.
Comment 2 Ryosuke Niwa 2010-12-05 17:46:32 PST
Created attachment 75640 [details]
fixes the bug
Comment 3 Tony Chang 2010-12-06 10:07:22 PST
Comment on attachment 75640 [details]
fixes the bug

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

> WebCore/editing/FormatBlockCommand.h:48
> -    void formatRange(const Position&, const Position&, RefPtr<Element>&);
> +    void formatRange(const Position&, const Position&, const Position& endOfSelection, RefPtr<Element>&);

Nit: Should we name the first and second Position params like in ApplyBlockElementCommand.h?

> WebCore/editing/IndentOutdentCommand.h:56
> -    void formatRange(const Position&, const Position&, RefPtr<Element>& blockquoteForNextIndent);
> +    void formatRange(const Position&, const Position&, const Position& endOfSelection, RefPtr<Element>& blockquoteForNextIndent);

Nit: Should we name the first and second Position params like in ApplyBlockElementCommand.h?
Comment 4 Ryosuke Niwa 2010-12-06 16:16:09 PST
Thanks for the review.

(In reply to comment #3)
> (From update of attachment 75640 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75640&action=review
> 
> > WebCore/editing/FormatBlockCommand.h:48
> > -    void formatRange(const Position&, const Position&, RefPtr<Element>&);
> > +    void formatRange(const Position&, const Position&, const Position& endOfSelection, RefPtr<Element>&);
> 
> Nit: Should we name the first and second Position params like in ApplyBlockElementCommand.h?
> 
> > WebCore/editing/IndentOutdentCommand.h:56
> > -    void formatRange(const Position&, const Position&, RefPtr<Element>& blockquoteForNextIndent);
> > +    void formatRange(const Position&, const Position&, const Position& endOfSelection, RefPtr<Element>& blockquoteForNextIndent);
> 
> Nit: Should we name the first and second Position params like in ApplyBlockElementCommand.h?

Will fix and land.
Comment 5 Ryosuke Niwa 2010-12-06 16:31:49 PST
Committed r73411: <http://trac.webkit.org/changeset/73411>