Summary: | Deleting when in front of a block image removes character from previous paragraph | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Garcia <justin.garcia> | ||||||
Component: | HTML Editing | Assignee: | Justin Garcia <justin.garcia> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | adele, enrica, eric, rniwa | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Justin Garcia
2009-04-27 22:13:56 PDT
Created attachment 42310 [details]
patch
patch and layout tests
With the test case, I see the image disappear, not a character from the previous line. Ryosuke pointed out that the layout tests were missing a newline at the end. Fixed that. Comment on attachment 42310 [details] patch > + if (isRenderedAsNonInlineTableImageOrHR(startOfParagraphToMove.deepEquivalent().node()) && !isStartOfParagraph(mergeDestination)) { > + m_endingPosition = m_upstreamStart; > + return; Why do we care whether the destination is the start of paragraph or not? > +bool isRenderedAsNonInlineTableImageOrHR(const Node* node) > +{ > + if (!node) > + return false; > + RenderObject* renderer = node->renderer(); > + return renderer && ((renderer->isTable() && !renderer->isInline()) || renderer->isImage() && !renderer->isInline() || renderer->isHR()); ... > -static bool renderedAsNonInlineTableOrHR(RenderObject* renderer) > -{ > - return renderer && ((renderer->isTable() && !renderer->isInline()) || renderer->isHR()); ... > - if (renderedAsNonInlineTableOrHR(startNode->renderer()) && p.atLastEditingPositionForNode()) > + if (isRenderedAsNonInlineTableImageOrHR(startNode)) Why are we removing the condition that p.atLastEditingPositionForNode()? I thought we were moving away from naming the tests after the bug number. I think it is better to give the test a meaningful name. > Why do we care whether the destination is the start of paragraph or not? If it is at the start of a paragraph, then the block table, image, or hr can move up (we can merge). I will add a comment about this. > Why are we removing the condition that p.atLastEditingPositionForNode()? Because firstDeepEditingPositionForNode(startNode) is also the start of the paragraph if p.atFirstEditingPositionForNode(). > I think it is better to give the test a meaningful name.
Will fix.
Created attachment 42445 [details]
patch
updated patch
|