[Webkit-unassigned] [Bug 51382] [Chromium]Issue 6024008: Consider the popup window position when the window shows upward

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=51382





--- Comment #18 from Ilya Sherman <isherman at chromium.org>  2010-12-22 14:56:55 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=77089&action=review

I'm having some trouble following these changes -- it looks like the patch you've uploaded is actually a combination of two diffs: (a) a diff of the second patch to the first, and (b) a diff of the first patch to the trunk version.  Please try again to upload the patch, and double-check that the diff looks right.  If you run into any trouble, probably the quickest way to get help is to hop onto #webkit on IRC, though of course posting questions here or via email also works.

> WebCore/ChangeLog:5
> +        [Chromium]Issue 6024008: Consider the popup window position when the window shows upward

nit: Please refer to the issue number in the Chromium bug tracker, and include a link: http://crbug.com/60427

For the description, I think something like this would be clearer: "Fix popup menu re-positioning when the menu is opened upward, above the corresponding form field."

> WebCore/ChangeLog:14
> +        * platform/chromium/PopupMenuChromium.cpp:
> +        (WebCore::PopupContainer::calcWidgetRect):
> +        (WebCore::PopupContainer::showPopup):
> +        (WebCore::PopupContainer::moveAndLayout):
> +        * platform/chromium/PopupMenuChromium.h:

nit: Please add brief descriptions of the changes for each of these.

> WebCore/platform/chromium/PopupMenuChromium.h:199
> +    IntRect calcWidgetRect(const IntRect& targetFrameRect);

nit: Please include a comment describing what this function does.  Also, prefer the full "calculate" to the abbreviated "calc".

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.