Bug 24066 – Chromium drop-down box: selected items don't paint correctly when scrolled

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Jay Campan]

When opening a select input in Chromium and scrolling-down in the popup, the items in the list don't paint properly, several are displayed as selected.

 Chromium associated bug : http://crbug.com/6743

[Jay Campan]

Created attachment 27835 [details]
FramelessScrollView::invalidateRect now invalidates in the scrolled view coordinates

A previous patch of mine fixing a painting problem with scroll-bars changes FramelessScrollView::invalidateRect to invalidate in the host window coordinates. This caused other painting problems.
This patch reverts back that changes and fixes the painting of scroll-bars by ensuring the invalidateScrollBars method invalidates in the scrolled view coordinates.

[Darin Fisher (:fishd, Google)]

Comment on attachment 27835 [details]
FramelessScrollView::invalidateRect now invalidates in the scrolled view coordinates

>[phone]  Jay Campan  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+

nit: please add a bug link here and a description of the code change.


>Index: WebCore/platform/chromium/FramelessScrollView.cpp
>===================================================================
>--- WebCore/platform/chromium/FramelessScrollView.cpp	(revision 41110)
>+++ WebCore/platform/chromium/FramelessScrollView.cpp	(working copy)
>@@ -47,7 +47,7 @@ void FramelessScrollView::invalidateScro
>     // Add in our offset within the ScrollView.
>     IntRect dirtyRect = rect;
>     dirtyRect.move(scrollbar->x(), scrollbar->y());
>-    invalidateRect(dirtyRect);
>+    invalidateRect(windowToContents(dirtyRect));

I would expect that we need to convert from the coordinates of the
Scrollbar to the coordinates of the ScrollView's content area.

That is what FrameView::invalidateScrollbarRect does, and it is what
the old code did.  So, this change seems wrong.


> bool FramelessScrollView::isActive() const
>@@ -59,7 +59,7 @@ bool FramelessScrollView::isActive() con
> void FramelessScrollView::invalidateRect(const IntRect& rect)
> {
>     if (HostWindow* h = hostWindow())
>-        h->repaint(rect, true);
>+        h->repaint(contentsToWindow(rect), true);

Same deal here.  I'm surprised to see this differ from FrameView's
invalidateRect implementation, but maybe there is something more
subtle at play?

[Jay Campan]

Created attachment 27966 [details]
Updated patch that invalidates in the parent coordinates.

[Darin Fisher (:fishd, Google)]

Comment on attachment 27966 [details]
Updated patch that invalidates in the parent coordinates.

>Index: WebCore/ChangeLog
...
>+        Reviewed by NOBODY (OOPS!).
>+
>+	https://bugs.webkit.org/show_bug.cgi?id=24066
>+
>+	Items in drop-downs were not painted correctly.
>+	Makes sure the PopupListBox invalidates in the coordinates of
>+	the window as this is FramelessScrollView::invalidateRect
>+	paints to.

the ChangeLog has tabs in it.  those should be replaced with spaces.

otherwise, looks great!

[Jay Campan]

Created attachment 27967 [details]
Updated patch, evil tabs removed in ChangeLog

[Darin Fisher (:fishd, Google)]

Landed as http://trac.webkit.org/changeset/41212