Bug 110618 – [Chromium] Add expandedToParagraph() method to WebRange

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Rouslan Solomakhin]

Add paragraphAroundCaret() method to WebFrame

[Rouslan Solomakhin]

Created attachment 189785 [details]
Patch

[Rouslan Solomakhin]

Groby, Tony: PTAL.

[Rouslan Solomakhin]

Groby: This method allows to query the spelling context. As you mentioned, we would not want to pass the spelling context up together with context click data. Another possibility would be add expandParagraph() method to WebRange object, but its comment says that WebRange is intended to be read-only.

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Tony Chang]

(In reply to comment #3)
> Groby: This method allows to query the spelling context. As you mentioned, we would not want to pass the spelling context up together with context click data. Another possibility would be add expandParagraph() method to WebRange object, but its comment says that WebRange is intended to be read-only.

Could you have WebRange::expandParagraph return a new WebRange with the extra text?

[Rouslan Solomakhin]

I certainly could! Let me get that done...

[Rouslan Solomakhin]

Created attachment 189859 [details]
Patch

[Rouslan Solomakhin]

Tony: PTAL.

[Tony Chang]

Comment on attachment 189859 [details]
Patch

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

> Source/WebKit/chromium/src/WebRange.cpp:101
> +    ExceptionCode ec;
> +    copy.m_private->expand("block", ec);

I think you want to use IGNORE_EXCEPTION here.

[Tony Chang]

(In reply to comment #9)
> (From update of attachment 189859 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189859&action=review
> 
> > Source/WebKit/chromium/src/WebRange.cpp:101
> > +    ExceptionCode ec;
> > +    copy.m_private->expand("block", ec);
> 
> I think you want to use IGNORE_EXCEPTION here.

LGTM with that change, but one of the API owners needs to approve.

[Adam Barth]

Comment on attachment 189859 [details]
Patch

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

> Source/WebKit/chromium/public/WebRange.h:76
> +    WEBKIT_EXPORT WebRange expandParagraph() const;

It looks like the WebCore version mutates the Rang whereas this one returns a new range that has been expanded.  Perhaps we should pick a name like "expandedToParagraph" to make it clear that we're not mutating the object.

[Rouslan Solomakhin]

Created attachment 190073 [details]
Patch for landing

[Rouslan Solomakhin]

Comment on attachment 190073 [details]
Patch for landing

Adam: PTAL.

[Adam Barth]

Comment on attachment 190073 [details]
Patch for landing

I should ask you for a test.  :)

[WebKit Review Bot]

Comment on attachment 190073 [details]
Patch for landing

Clearing flags on attachment: 190073

Committed r144002: <http://trac.webkit.org/changeset/144002>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.