Bug 25439 – Deleting when in front of a block image removes character from previous paragraph

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Justin Garcia]

Use:

<div contentEditable="true">hello<img style="display:block;" src="http://www.google.com/logos/logo.gif"></div>

Put the caret in front of the image and hit delete.  Caret should probably just move back one, instead of deleting a character from the previous paragraph.

[Justin Garcia]

<rdar://problem/6767162>

[Justin Garcia]

Created attachment 42310 [details]
patch

patch and layout tests

[Adele Peterson]

With the test case, I see the image disappear, not a character from the previous line.

[Justin Garcia]

Ryosuke pointed out that the layout tests were missing a newline at the end.  Fixed that.

[Ryosuke Niwa]

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()?

[Enrica Casucci]

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.

[Justin Garcia]

> 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().

[Justin Garcia]

> I think it is better to give the test a meaningful name.

Will fix.

[Justin Garcia]

Created attachment 42445 [details]
patch

updated patch

[Justin Garcia]

http://trac.webkit.org/changeset/50542