Bug 138567

Summary:

Some functions on DictionaryLookup.h should just be generic functions on Range or VisiblePosition

Product:

WebKit

Reporter:

Beth Dakin <bdakin>

Component:

HTML Editing

Assignee:

Tim Horton <thorton>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, enrica, simon.fraser, thorton

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Beth Dakin 2014-11-10 11:12:28 PST

Some functions on DictionaryLookup.h should just be generic functions on Range or VisiblePosition, specifically isPositionInRange() and shouldUseSelection()

Comment 1 Beth Dakin 2014-11-10 11:14:17 PST

And rangeExpandedAroundPositionByCharacters()! Which is code originally stolen from Editor, according to Tim.

Comment 2 Tim Horton 2015-08-17 14:47:30 PDT

Created attachment 259191 [details]
Patch

Comment 3 Tim Horton 2015-08-17 19:03:16 PDT

http://trac.webkit.org/changeset/188565

Comment 4 Said Abou-Hallawa 2015-08-18 09:03:47 PDT

Comment on attachment 259191 [details]
Patch

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

> Source/WebCore/dom/Range.cpp:1963
> +bool Range::contains(const VisiblePosition& position) const

Can't we be more generic here and have the prototype of this function: bool Range::contains(const VisiblePosition &start, const VisiblePosition &end)?

> Source/WebCore/editing/mac/DictionaryLookup.mm:61
> +    return selectedRange->contains(position);

To check whether a VisiblePosition is in a range of VisibleSelection, is not it sufficient to call Range::isPointInRange()?
What is the difference if VisiblePosition is at the boundary of the range of VisibleSelection when:
1. calling Range::isPointInRange()
2. calling the deleted function isPositionInRange()
3. calling Range::contains()