Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 6532004: DO NOT SUBMIT (Closed)

Created:
9 years, 10 months ago by Robert Sesek
Modified:
9 years, 6 months ago
Reviewers:
James Su
CC:
chromium-reviews
Base URL:
git://git.webkit.org/WebKit.git@master
Visibility:
Public.

Description

DO NOT SUBMIT WebKit side of a two-sided patch for the Mac Lookup In Dictionary functionality. The Chromium side is here: http://codereview.chromium.org/6289009/ BUG= https://bugs.webkit.org/show_bug.cgi?id=54969

Patch Set 1 #

Patch Set 2 : Migrate WebTextHelper #

Total comments: 7

Patch Set 3 : Address feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+363 lines, -62 lines) Patch
M Source/WebCore/dom/Range.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M Source/WebCore/dom/Range.cpp View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
M Source/WebCore/page/Frame.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/WebCore/page/Frame.cpp View 1 2 1 chunk +25 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/WebKit.gyp View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/public/WebFrame.h View 1 2 2 chunks +8 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/public/WebWidget.h View 1 2 3 chunks +10 lines, -0 lines 0 comments Download
A Source/WebKit/chromium/public/mac/WebTextHelper.h View 1 2 1 chunk +58 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/src/WebFrameImpl.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/WebKit/chromium/src/WebFrameImpl.cpp View 1 2 3 chunks +22 lines, -1 line 0 comments Download
M Source/WebKit/chromium/src/WebPopupMenuImpl.h View 1 2 2 chunks +3 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/src/WebPopupMenuImpl.cpp View 1 2 3 chunks +16 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/src/WebViewImpl.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M Source/WebKit/chromium/src/WebViewImpl.cpp View 1 2 3 chunks +38 lines, -0 lines 0 comments Download
A Source/WebKit/chromium/src/mac/WebTextHelper.mm View 1 1 chunk +136 lines, -0 lines 0 comments Download
M Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M Source/WebKit2/WebProcess/WebPage/WebPage.h View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M Source/WebKit2/WebProcess/WebPage/WebPage.cpp View 1 2 1 chunk +0 lines, -29 lines 0 comments Download
M Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm View 1 2 3 chunks +6 lines, -29 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
James Su
http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/public/WebWidget.h File Source/WebKit/chromium/public/WebWidget.h (right): http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/public/WebWidget.h#newcode125 Source/WebKit/chromium/public/WebWidget.h:125: virtual WebRange compositionRange() = 0; We should convert the ...
9 years, 9 months ago (2011-03-03 04:08:03 UTC) #1
Robert Sesek
9 years, 9 months ago (2011-03-18 21:33:36 UTC) #2 
Addressed your feedback and upstream feedback. I'll move these changes upstream
on Monday.

http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/publi...
File Source/WebKit/chromium/public/WebWidget.h (right):

http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/publi...
Source/WebKit/chromium/public/WebWidget.h:125: virtual WebRange
compositionRange() = 0;
On 2011/03/03 04:08:03, James Su wrote:
> We should convert the result range into absolute character index and length
> relative to the document. WebRange.startOffset() and endOffset() return the
> index inside its start and end node.

Done.

http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/publi...
Source/WebKit/chromium/public/WebWidget.h:137: virtual WebRange
caretOrSelectionRange() = 0;
On 2011/03/03 04:08:03, James Su wrote:
> Same as above.

Done.

http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/src/W...
File Source/WebKit/chromium/src/WebFrameImpl.cpp (right):

http://codereview.chromium.org/6532004/diff/1001/Source/WebKit/chromium/src/W...
Source/WebKit/chromium/src/WebFrameImpl.cpp:1119: rect =
frame()->view()->contentsToWindow(rect);
On 2011/03/03 04:08:03, James Su wrote:
> IMHO, we should return frame based position in WebFrame implementation.
> And add functions that return window based position into WebWidget/WebView.

Why? That seems unnecessary and makes this CL more complicated than it needs to
be.

Powered by Google App Engine
This is Rietveld 408576698