[webkit-reviews] review denied: [Bug 78730] Initial upstreaming of EditorClientBlackBerry : [Attachment 127217] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Mike Fenton <mifenton at rim.com>'s
request for review:
Bug 78730: Initial upstreaming of EditorClientBlackBerry
https://bugs.webkit.org/show_bug.cgi?id=78730

Attachment 127217: Patch
https://bugs.webkit.org/attachment.cgi?id=127217&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=127217&action=review


Looks good, can be cleaned up a bit more.

> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.cpp:168
> +	   return
m_webPagePrivate->m_dumpRenderTree->shouldEndEditingInDOMRange(range);

We are a bit inconsistent in usage of newlines here, compare to
shouldBeginEditing. We should probably choose one style within a file.

> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.cpp:189
> +	   return
m_webPagePrivate->m_dumpRenderTree->shouldChangeSelectedDOMRangeToDOMRangeAffin
ityStillSelecting(fromRange, toRange, static_cast<int>(affinity),
stillSelecting);

I wonder if the early return is ok here but that is something to investigate in
a seperate bug.

> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.h:63
> +    virtual void registerCommandForUndo(WTF::PassRefPtr<EditCommand>);

I think you can drop all WTF prefixes in the header.