at line 150 of ScrollbarThemeQt.cpp there is the followin instruction: p.painter->setClipRect(opt->rect.intersected(damageRect)); this causes the following problem: a QGraphicsWidget renders a QWebPage is the children of another qgw wit the flag ItemClipsChildrenToShape set. the scrollbars will be painted not clipped to the parent shape because of the cliprect reset. this patch intersect the rect, with the previous one, making the scrollbar to be correctly clipped
Created attachment 41185 [details] patch that corrects the bug
(In reply to comment #1) > Created an attachment (id=41185) [details] > patch that corrects the bug Good catch! Marco, your patch looks correct, but it needs to be adapted to the latest code changes in the trunk - the code is now in ScrollbarThemeQt.cpp - and your patch needs a ChangeLog entry. Please see http://webkit.org/coding/contributing.html
Created attachment 44478 [details] updated patch updated the patch to the new filename and added a ChangeLog entry
Is this supposed to be up for review? If so, please set the review flag to r?
Comment on attachment 44478 [details] updated patch > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 51868) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,19 @@ > +2009-12-08 Marco Martin <notmart@gmail.com> > + > + Review pending. > + > + In the Scrollbar painting of the Qt theme integration, > + intersects any previous clip rects with the one needed to paint the scrollbar. > + This fixes the painting in QGraphicsview, when the parent of the widget that > + paints the QWebPage has the QGraphicsItem::ItemClipsChildrenToShape set and > + a piece of the scrollbar should be cipped away > + https://bugs.webkit.org/show_bug.cgi?id=30366 > + > + No new tests. > + > + * platform/qt/ScrollbarThemeQt.cpp: > + (WebCore::ScrollbarThemeQt::paint): > + > 2009-12-08 John Gregg <johnnyg@google.com> > > Reviewed by Adam Barth. > Index: WebCore/platform/qt/ScrollbarThemeQt.cpp > =================================================================== > --- WebCore/platform/qt/ScrollbarThemeQt.cpp (revision 51865) > +++ WebCore/platform/qt/ScrollbarThemeQt.cpp (working copy) > @@ -147,7 +147,7 @@ > p.painter->save(); > QStyleOptionSlider* opt = styleOptionSlider(scrollbar, p.widget); > > - p.painter->setClipRect(opt->rect.intersected(damageRect)); > + p.painter->setClipRect(opt->rect.intersected(damageRect), Qt::IntersectClip); > > #ifdef Q_WS_MAC > p.drawComplexControl(QStyle::CC_ScrollBar, *opt);
@Kenneth: yes, the patch should be final i think (apart the revieved by in the changelog :))
Comment on attachment 44478 [details] updated patch > +2009-12-08 Marco Martin <notmart@gmail.com> > + > + Review pending. > + You need to leave the review line untouched, unless you want to land this manually, but as I believe you do not have commit rights, someone else could add it to the commit-queue. Can you reupload?
Comment on attachment 44478 [details] updated patch Marco is not a reviewer, clearly r+. I assume you meant to mark this as r? instead?
I meant "clearing" instead of "clearly", sorry.
Comment on attachment 44478 [details] updated patch > + Review pending. Don't change the line made by prepare-Changelog > + No new tests. Would be good with a test, at least a manual one (goes in WebCore/manual-tests).
The Review Patch link is probably not the link you want. "Details" is more useful. Review Patch is kinda a lame hack that just adds the full diff as a comment and shows you the "Formatted Diff" view in an iframe.
Created attachment 44821 [details] second patch update I've done the change in the changelog, hope is correct now.
Comment on attachment 44821 [details] second patch update > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 52117) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,19 @@ > +2009-12-14 Marco Martin <notmart@gmail.com> > + > + Reviewed by NOBODY (OOPS!). > + > + In the Scrollbar painting of the Qt theme integration, > + intersects any previous clip rects with the one needed to paint the scrollbar. > + This fixes the painting in QGraphicsview, when the parent of the widget that > + paints the QWebPage has the QGraphicsItem::ItemClipsChildrenToShape set and > + a piece of the scrollbar should be cipped away > + https://bugs.webkit.org/show_bug.cgi?id=30366 > + > + No new tests. > + > + * platform/qt/ScrollbarThemeQt.cpp: > + (WebCore::ScrollbarThemeQt::paint): > + > 2009-12-14 Alexey Proskuryakov <ap@apple.com> > > Reviewed by Dave Hyatt. > Index: WebCore/platform/qt/ScrollbarThemeQt.cpp > =================================================================== > --- WebCore/platform/qt/ScrollbarThemeQt.cpp (revision 51865) > +++ WebCore/platform/qt/ScrollbarThemeQt.cpp (working copy) > @@ -147,7 +147,7 @@ > p.painter->save(); > QStyleOptionSlider* opt = styleOptionSlider(scrollbar, p.widget); > > - p.painter->setClipRect(opt->rect.intersected(damageRect)); > + p.painter->setClipRect(opt->rect.intersected(damageRect), Qt::IntersectClip); > > #ifdef Q_WS_MAC > p.drawComplexControl(QStyle::CC_ScrollBar, *opt);
Comment on attachment 44821 [details] second patch update LGTM, but please look into adding a test. Thanks for the work Martin
in manual-tests i see only html files, since this one is not a rendering problem it would be more useful a short program... i'll look into that. how they should be done?
Ah ofcourse, sorry. Looking at too many patches :-) I don't know the best way to actually add a test for this that can be tested automatically. The closest would be a Qt autotest, which we use for testing our APIs.
Comment on attachment 44821 [details] second patch update Clearing flags on attachment: 44821 Committed r52122: <http://trac.webkit.org/changeset/52122>
All reviewed patches have been landed. Closing bug.