[webkit-reviews] review granted: [Bug 217895] REGRESSION(r266295): Range allows start and end containers to belong to different trees : [Attachment 411953] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 217895: REGRESSION(r266295): Range allows start and end containers to
belong to different trees
https://bugs.webkit.org/show_bug.cgi?id=217895

Attachment 411953: Patch

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




--- Comment #20 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 411953
  --> https://bugs.webkit.org/attachment.cgi?id=411953
Patch

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

Should I file a new bug for DOMSelection?

> Source/WebCore/ChangeLog:26
> +	   also be objects of another type. Maybe an enumeration named TreeType
instead?

Maybe enum class would be more appropriate.

> Source/WebCore/dom/Node.h:759
> +struct ShadowIncludingTree { };

So it never makes sense to use this type thing for ordering nodes.
It's only ever used when we're walking up the ancestor nodes so maybe
TreeIncludingShadowAncestors might be appropriate.
We probably don't want to support it in treeOrder though.

> Source/WebCore/dom/SimpleRange.cpp:215
> +template<typename TreeType> bool isPointInRange(const SimpleRange& range,
const BoundaryPoint& point)

This is somewhat tangential but this seems backwards.
I'd have expected point to come before range given this is asking: is *point*
in *range*.