Bug 47300 - Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
Summary: Executing FormatBlock on multiple paragraphs inside pre does not remove the o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-06 13:53 PDT by Ryosuke Niwa
Modified: 2010-12-06 16:31 PST (History)
6 users (show)

See Also:


Attachments
demo (396 bytes, text/html)
2010-10-06 13:53 PDT, Ryosuke Niwa
no flags Details
fixes the bug (18.23 KB, patch)
2010-12-05 17:46 PST, Ryosuke Niwa
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>