Bug 86670

Summary:

[chromium] plumb the frame for which a drag was initiated to the WebViewClient

Product:

WebKit

Reporter:

jochen

Component:

New Bugs

Assignee:

jochen

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, fishd, jamesr, tkent, tkent+wkapi, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

jochen

Reported 2012-05-16 13:32:44 PDT

[chromium] plumb the frame for which a drag was initiated to the WebViewClient

Attachments
Patch (4.37 KB, patch) 2012-05-16 13:33 PDT, jochen	no flags	Details Formatted Diff Diff
Patch (6.96 KB, patch) 2012-05-16 13:47 PDT, jochen	no flags	Details Formatted Diff Diff
Patch (6.96 KB, patch) 2012-05-18 00:18 PDT, jochen	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

jochen

Comment 1 2012-05-16 13:33:02 PDT

Created attachment 142334 [details] Patch

WebKit Review Bot

Comment 2 2012-05-16 13:35:15 PDT

Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

jochen

Comment 3 2012-05-16 13:37:03 PDT

I intend to use the Frame to get its document's referrer policy, so it can be used for when the drag results in a download

jochen

Comment 4 2012-05-16 13:47:26 PDT

Created attachment 142337 [details] Patch

James Robinson

Comment 5 2012-05-16 14:44:30 PDT

Comment on attachment 142337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142337&action=review Left some comments on how to stage this, but I don't have any insight about the change itself. > Source/WebKit/chromium/public/WebViewClient.h:254 > + // FIXME: Remove once the WebKit side has landed. I'm confused by this comment - this patch is the WebKit side, isn't it? Did you mean chromium side? One way to do this without waiting for rolls is to guard both sides of the change in an #ifdef like WEBVIEWCLIENT_STARTDRAGGING_HAS_FRAME and #define that in this header, then remove the #ifdefs/#defines after everything lands and rolls.

jochen

Comment 6 2012-05-18 00:18:26 PDT

Created attachment 142651 [details] Patch

jochen

Comment 7 2012-05-18 00:21:05 PDT

(In reply to comment #5) > (From update of attachment 142337 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142337&action=review > > Left some comments on how to stage this, but I don't have any insight about the change itself. > > > Source/WebKit/chromium/public/WebViewClient.h:254 > > + // FIXME: Remove once the WebKit side has landed. > > I'm confused by this comment - this patch is the WebKit side, isn't it? Did you mean chromium side? Yes > One way to do this without waiting for rolls is to guard both sides of the change in an #ifdef like WEBVIEWCLIENT_STARTDRAGGING_HAS_FRAME and #define that in this header, then remove the #ifdefs/#defines after everything lands and rolls. Your approach requires 4 patches, mine only three (or 6 vs 5 if you include rolls) The advantage of your approach is that you can do breaking changes, e.g. introduce a new enum value. But for just changing a signature or similar, my approach seems to be the more common one

Kent Tamura

Comment 8 2012-05-18 00:33:23 PDT

Comment on attachment 142651 [details] Patch looks good.

WebKit Review Bot

Comment 9 2012-05-18 01:11:19 PDT

Comment on attachment 142651 [details] Patch Clearing flags on attachment: 142651 Committed r117560: <http://trac.webkit.org/changeset/117560>

WebKit Review Bot

Comment 10 2012-05-18 01:11:24 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.