[Webkit-unassigned] [Bug 39989] Applying inline style to a text node whose parent is an inline editable root causes crash

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62586|review?                     |review+
               Flag|                            |




--- Comment #13 from Darin Adler <darin at apple.com>  2010-07-26 13:55:11 PST ---
(From update of attachment 62586)
> +    Node* parent = start.node()->parentNode();
> +    if (parent && parent->parentElement() && parent->parentElement()->isContentEditable()) {
> +        int endOffsetAdjustment = start.node() == end.node() ? start.deprecatedEditingOffset() : 0;
> +        Text* text = static_cast<Text*>(start.node());
> +        splitTextNodeContainingElement(text, start.deprecatedEditingOffset());
> +        updateStartEnd(Position(start.node()->parentNode(), start.node()->nodeIndex()), Position(end.node(), end.deprecatedEditingOffset() - endOffsetAdjustment));
> +    } else {
> +        splitTextAtStart(start, end);
> +    }

In WebKit we often take the "early exit" approach, checking for an unusual case and handling that case, then using a return. The non-unusual case then continues without be indented inside an if statement. Doing that here would have two benefits besides the usual ones:

    1) The diff would be smaller because we wouldn't be re-indenting all the code.
    2) The body of the if statement would be two lines so you could use braces around it, unlike the one line else body.

It’s not WebKit style to have braces around a 1-line else body, but such things are pretty ugly without the braces, so I think it would be nice to avoid it.

Patch otherwise looks fine, but I think it would be better with early exit.

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