[webkit-reviews] review granted: [Bug 192613] Make HTMLConverter take two Positions in preparation to make it work with shadow DOM : [Attachment 357099] Cleanup

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 192613: Make HTMLConverter take two Positions in preparation to make it
work with shadow DOM
https://bugs.webkit.org/show_bug.cgi?id=192613

Attachment 357099: Cleanup

https://bugs.webkit.org/attachment.cgi?id=357099&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 357099
  --> https://bugs.webkit.org/attachment.cgi?id=357099
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=357099&action=review

Wish we had a structure with two Positions instead of two separate ones.

If we take two positions, do we need to take precautions against traversing the
entire document if end is not after start? Range takes care of that, but two
positions don’t.

> Source/WebCore/editing/cocoa/HTMLConverter.mm:2459
> +    HTMLConverter converter(range.startPosition(), range.endPosition());

I would have used { } syntax. And maybe not named the local variable.

> Source/WebCore/editing/cocoa/HTMLConverter.mm:2467
> +    HTMLConverter converter(range->startPosition(), range->endPosition());

Ditto.