[chromium] REGRESSION: Refreshed autofill popup renders garbage

• Component: Forms
• Status: RESOLVED FIXED

[Alexander Pavlov (apavlov)]

What steps will reproduce the problem?
1. Enter a very long string into autofillable form field and submit form
2. Repeat process and choose very long value from drop down
3. You may need to focus on new field or effect may become immediately visible (see attached image)

What is the expected result?
- Drop down with value (truncated?)
- Form field filled with value, autofill drop down disappears

What happens instead?
- Drop down stretches across page
- When selected dropdown moves to below the address bar followed by a badly rendered portion of the page.

Upstreaming http://code.google.com/p/chromium/issues/detail?id=118374

[Alexander Pavlov (apavlov)]

The issue is due to the incorrect layout/painting of a refreshed popup that contains a PopupListBox that does not fit into the screen width. In this case, the WebWidget - PopupContainer - PopupListBox hierarchy is painted incorrectly: the PopupContainer is given a position relative to the containing window, not the WebWidget (through chromeClient->setWindowRect()), thereby making it paint iself outside of the WebWidget (which results in the garbage), and chromeClient is never notified of the popup relocation.

[Alexander Pavlov (apavlov)]

Created attachment 135798 [details]
Patch

[Ilya Sherman]

Comment on attachment 135798 [details]
Patch

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

I am not a reviewer, but this looks good to me.  Thanks for the fix!

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=83255

nit: Please add a link to the Chromium bug here as well.

> Source/WebCore/ChangeLog:16
> +        (PopupContainer):

nit: Please briefly describe the changes to each file.

> Source/WebCore/platform/chromium/PopupContainer.cpp:414
> +

nit: I would omit this blank line; but if you prefer to keep it, that's fine too.

[Kent Tamura]

Comment on attachment 135798 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +
> +        No new tests, as the popup appearance is not testable in WebKit.

It would be nice if you explain what was wrong and how do you fix it in ChangeLog.

> Source/WebCore/platform/chromium/PopupContainer.cpp:202
> +    // In screen coordinates.
>      return widgetRect;

Renaming widgetRect to widgetRectInScreen might be better.

> Source/WebCore/platform/chromium/PopupContainer.cpp:418
> +    // In window coordinates.
>      IntPoint location = m_frameView->contentsToWindow(targetControlRect.location());

ditto.  locationInWindow

> Source/WebCore/platform/chromium/PopupContainer.cpp:424
> +    // In screen coordinates.
>      IntRect widgetRect = layoutAndCalculateWidgetRect(targetControlRect.height(), location);

ditto. widgetRectInScreen

[Alexander Pavlov (apavlov)]

Committed r113418: <http://trac.webkit.org/changeset/113418>

[Alexander Pavlov (apavlov)]

Landed with all the comments addressed.