Bug 19644 – Text copied with Select All pastes with a indent but shouldn't

• Component: HTML Editing
• Status: RESOLVED FIXED

[Justin Garcia]

Open FCKEditor http://www.fckeditor.net/demo (this also happens in a few others)
Select all of the default content
Delete
Paste

Content pastes indented slightly.  It shouldn't.  Also, press return and paste again.  You'll see that the content is indented even more.

<rdar://problem/5581805> Text copied with Select All pastes with a indent but shouldn't

[Justin Garcia]

Created attachment 29641 [details]
patch

[Darin Adler]

Comment on attachment 29641 [details]
patch

Ideally I'd also like to see a check if the destination already has the properties in question. For example, the same background color or same image.

> +        if (fullySelectedRootStyle->getPropertyCSSValue(CSSPropertyBackgroundImage) ||
> +            fullySelectedRootStyle->getPropertyCSSValue(CSSPropertyBackgroundColor) ||
> +            static_cast<Element*>(fullySelectedRoot)->hasAttribute(backgroundAttr))

I find this if statement really difficult to read. I would prefer that you put it in a separate function or format it differently.

I don't understand why the cast to Element* here is safe. What guarantees that fullySelectedRoot is an Element, not another kind of Node?

Given that we're mutating the DOM here, maybe fullySelectedRoot should be a RefPtr<Node>, not just a Node*.

r=me

[Justin Garcia]

> I don't understand why the cast to Element* here is safe. What guarantees that
> fullySelectedRoot is an Element, not another kind of Node?

Currently it must be the body element, but since that's non obvious maybe I'll add a check.

> Given that we're mutating the DOM here, maybe fullySelectedRoot should be a
> RefPtr<Node>, not just a Node*.

Are we?  Does attribute checking fire listeners that can modify the DOM, or?

[Justin Garcia]

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