[Webkit-unassigned] [Bug 78193] Inserting a paragraph between quoted lines in editing/deleting/delete-4038408-fix.html doesn't work

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78193





--- Comment #10 from yi shen <yi.4.shen at nokia.com>  2012-04-02 12:12:01 PST ---
Thanks for the review. Will update the patch with your comments.
(In reply to comment #9)
> (From update of attachment 135138 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135138&action=review
> 
> Okay :( Sounds like we need this hack.
> 
> > Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:232
> > +            if (m_pasteBlockqutoeIntoUnquotedArea)
> > +                if (Node* highestBlockquote = highestEnclosingNodeOfType(canonicalPos, &isMailBlockquote))
> > +                    startBlock = static_cast<Element*>(highestBlockquote);
> 
> You need the curly brackets around the outer if.
> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1131
> >                  } else
> >                      // Use a default paragraph element (a plain div) for the empty paragraph, using the last paragraph
> >                      // block's style seems to annoy users.
> > -                    insertParagraphSeparator(true);
> > +                    insertParagraphSeparator(true, (!startIsInsideMailBlockquote && insertMailBlockquote));
> >  
> 
> We need curly brackets around this statement and comments since they span more than one line.
> 
> > LayoutTests/editing/inserting/insert-paragraph-separator-in-blockquote.html:30
> > +        var ev = document.createEvent("KeyboardEvent");
> 
> Please don't use abbreviations like ev.
> 
> > LayoutTests/editing/inserting/insert-paragraph-separator-in-blockquote.html:56
> > +if ((nodesOfTopDiv == topDiv.childNodes.length) && (blockquoteElement.childNodes.length == (nodesOfBlockquote + 1)))
> > +    document.body.innerHTML = "SUCCESS";
> > +else
> > +    document.body.innerHTML = "FAIL";
> 
> I don't think this is a good check. We may, in the future, generate slightly different markup and this test will break. I think it's better to use dump-as-markup.js.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.