Bug 192613 – Make HTMLConverter take two Positions in preparation to make it work with shadow DOM

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

In order to make HTMLConverter compatible with shadow DOM, we need to make it take two Positions instead of a Range
so that it can represent a selection which spans across shadow boundaries.

[Ryosuke Niwa]

Created attachment 357099 [details]
Cleanup

[Darin Adler]

Comment on attachment 357099 [details]
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.

[Ryosuke Niwa]

(In reply to Darin Adler from comment #2)
> Comment on attachment 357099 [details]
> 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.

That is a good point. We have an early return for that in serializePreservingVisualAppearanceInternal too:

if (!comparePositions(start, end))
    return emptyString();

We should probably update it to:

if (comparePositions(start, end) >= 0)
    return emptyString();

I'm gonna add the following early return to HTMLConverter::convert():
if (comparePositions(m_start, m_end) > 0)
    return nil;

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

Fixed.

[Ryosuke Niwa]

Committed r239127: <https://trac.webkit.org/changeset/239127>

[Radar WebKit Bug Importer]

<rdar://problem/46673257>