[webkit-reviews] review granted: [Bug 19644] Text copied with Select All pastes with a indent but shouldn't : [Attachment 29641] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 21 07:53:59 PDT 2009

Darin Adler <darin at apple.com> has granted Justin Garcia
<justin.garcia at apple.com>'s request for review:
Bug 19644: Text copied with Select All pastes with a indent but shouldn't

Attachment 29641: patch

------- Additional Comments from Darin Adler <darin at apple.com>
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) ||
> +	      

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*.


More information about the webkit-reviews mailing list