30366 2009-10-14 13:24:41 -0700 if the QWebPage is rendered by a QGraphicsWidget child of another widget with ItemClipsChildrenToShape the scrollbar is not clipped 2009-12-14 14:31:11 -0800 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC Linux RESOLVED FIXED Qt P2 Minor --- 0 notmart webkit-unassigned commit-queue eric hausmann kenneth notmart oldest_to_newest 154642 0 notmart 2009-10-14 13:24:41 -0700 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 154643 1 41185 notmart 2009-10-14 13:25:59 -0700 Created attachment 41185 patch that corrects the bug 167124 2 hausmann 2009-11-29 16:06:17 -0800 (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 169740 3 44478 notmart 2009-12-08 11:37:38 -0800 Created attachment 44478 updated patch updated the patch to the new filename and added a ChangeLog entry 171363 4 kenneth 2009-12-14 05:58:44 -0800 Is this supposed to be up for review? If so, please set the review flag to r? 171368 5 44478 notmart 2009-12-14 06:20:16 -0800 Comment on attachment 44478 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); 171369 6 notmart 2009-12-14 06:21:12 -0800 @Kenneth: yes, the patch should be final i think (apart the revieved by in the changelog :)) 171370 7 44478 kenneth 2009-12-14 06:26:39 -0800 Comment on attachment 44478 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? 171558 8 44478 eric 2009-12-14 13:52:31 -0800 Comment on attachment 44478 updated patch Marco is not a reviewer, clearly r+. I assume you meant to mark this as r? instead? 171559 9 eric 2009-12-14 13:52:52 -0800 I meant "clearing" instead of "clearly", sorry. 171560 10 44478 notmart 2009-12-14 13:54:49 -0800 Comment on attachment 44478 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); 171564 11 44478 kenneth 2009-12-14 13:58:33 -0800 Comment on attachment 44478 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). 171566 12 eric 2009-12-14 14:00:28 -0800 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. 171567 13 44821 notmart 2009-12-14 14:03:23 -0800 Created attachment 44821 second patch update I've done the change in the changelog, hope is correct now. 171568 14 44821 notmart 2009-12-14 14:03:44 -0800 Comment on attachment 44821 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); 171574 15 44821 kenneth 2009-12-14 14:09:47 -0800 Comment on attachment 44821 second patch update LGTM, but please look into adding a test. Thanks for the work Martin 171576 16 notmart 2009-12-14 14:10:58 -0800 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? 171578 17 kenneth 2009-12-14 14:14:38 -0800 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. 171584 18 44821 commit-queue 2009-12-14 14:31:05 -0800 Comment on attachment 44821 second patch update Clearing flags on attachment: 44821 Committed r52122: <http://trac.webkit.org/changeset/52122> 171585 19 commit-queue 2009-12-14 14:31:11 -0800 All reviewed patches have been landed. Closing bug. 41185 2009-10-14 13:25:59 -0700 2009-12-08 11:37:38 -0800 patch that corrects the bug clipscrollbar.diff text/plain 516 notmart SW5kZXg6IFNjcm9sbGJhclRoZW1lUXQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNjcm9sbGJhclRoZW1l UXQuY3BwCShyZXZpc2lvbiA0OTU2OCkKKysrIFNjcm9sbGJhclRoZW1lUXQuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC0xNDcsNyArMTQ3LDcgQEAKICAgICBwLnBhaW50ZXItPnNhdmUoKTsKICAgICBR U3R5bGVPcHRpb25TbGlkZXIqIG9wdCA9IHN0eWxlT3B0aW9uU2xpZGVyKHNjcm9sbGJhciwgcC53 aWRnZXQpOwogCi0gICAgcC5wYWludGVyLT5zZXRDbGlwUmVjdChvcHQtPnJlY3QuaW50ZXJzZWN0 ZWQoZGFtYWdlUmVjdCkpOworICAgIHAucGFpbnRlci0+c2V0Q2xpcFJlY3Qob3B0LT5yZWN0Lmlu dGVyc2VjdGVkKGRhbWFnZVJlY3QpLCBRdDo6SW50ZXJzZWN0Q2xpcCk7CiAKICNpZmRlZiBRX1dT X01BQwogICAgIHAuZHJhd0NvbXBsZXhDb250cm9sKFFTdHlsZTo6Q0NfU2Nyb2xsQmFyLCAqb3B0 KTsK 44478 2009-12-08 11:37:38 -0800 2009-12-14 14:03:23 -0800 updated patch intersectclip.diff text/plain 1473 notmart SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MTg2OCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMDktMTItMDggIE1hcmNvIE1hcnRpbiAgPG5vdG1hcnRAZ21haWwu Y29tPgorCisgICAgICAgIFJldmlldyBwZW5kaW5nLgorCisgICAgICAgIEluIHRoZSBTY3JvbGxi YXIgcGFpbnRpbmcgb2YgdGhlIFF0IHRoZW1lIGludGVncmF0aW9uLAorICAgICAgICBpbnRlcnNl Y3RzIGFueSBwcmV2aW91cyBjbGlwIHJlY3RzIHdpdGggdGhlIG9uZSBuZWVkZWQgdG8gcGFpbnQg dGhlIHNjcm9sbGJhci4KKyAgICAgICAgVGhpcyBmaXhlcyB0aGUgcGFpbnRpbmcgaW4gUUdyYXBo aWNzdmlldywgd2hlbiB0aGUgcGFyZW50IG9mIHRoZSB3aWRnZXQgdGhhdCAKKyAgICAgICAgcGFp bnRzIHRoZSBRV2ViUGFnZSBoYXMgdGhlIFFHcmFwaGljc0l0ZW06Okl0ZW1DbGlwc0NoaWxkcmVu VG9TaGFwZSBzZXQgYW5kCisgICAgICAgIGEgcGllY2Ugb2YgdGhlIHNjcm9sbGJhciBzaG91bGQg YmUgY2lwcGVkIGF3YXkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTMwMzY2CisKKyAgICAgICAgTm8gbmV3IHRlc3RzLgorCisgICAgICAgICogcGxhdGZv cm0vcXQvU2Nyb2xsYmFyVGhlbWVRdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTY3JvbGxiYXJU aGVtZVF0OjpwYWludCk6CisKIDIwMDktMTItMDggIEpvaG4gR3JlZ2cgIDxqb2hubnlnQGdvb2ds ZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KSW5kZXg6IFdlYkNvcmUv cGxhdGZvcm0vcXQvU2Nyb2xsYmFyVGhlbWVRdC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w bGF0Zm9ybS9xdC9TY3JvbGxiYXJUaGVtZVF0LmNwcAkocmV2aXNpb24gNTE4NjUpCisrKyBXZWJD b3JlL3BsYXRmb3JtL3F0L1Njcm9sbGJhclRoZW1lUXQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0x NDcsNyArMTQ3LDcgQEAKICAgICBwLnBhaW50ZXItPnNhdmUoKTsKICAgICBRU3R5bGVPcHRpb25T bGlkZXIqIG9wdCA9IHN0eWxlT3B0aW9uU2xpZGVyKHNjcm9sbGJhciwgcC53aWRnZXQpOwogCi0g ICAgcC5wYWludGVyLT5zZXRDbGlwUmVjdChvcHQtPnJlY3QuaW50ZXJzZWN0ZWQoZGFtYWdlUmVj dCkpOworICAgIHAucGFpbnRlci0+c2V0Q2xpcFJlY3Qob3B0LT5yZWN0LmludGVyc2VjdGVkKGRh bWFnZVJlY3QpLCBRdDo6SW50ZXJzZWN0Q2xpcCk7CiAKICNpZmRlZiBRX1dTX01BQwogICAgIHAu ZHJhd0NvbXBsZXhDb250cm9sKFFTdHlsZTo6Q0NfU2Nyb2xsQmFyLCAqb3B0KTsK 44821 2009-12-14 14:03:23 -0800 2009-12-14 14:31:05 -0800 second patch update intersectclip2.diff text/plain 1488 notmart SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MjExNykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMDktMTItMTQgIE1hcmNvIE1hcnRpbiAgPG5vdG1hcnRAZ21haWwu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIElu IHRoZSBTY3JvbGxiYXIgcGFpbnRpbmcgb2YgdGhlIFF0IHRoZW1lIGludGVncmF0aW9uLAorICAg ICAgICBpbnRlcnNlY3RzIGFueSBwcmV2aW91cyBjbGlwIHJlY3RzIHdpdGggdGhlIG9uZSBuZWVk ZWQgdG8gcGFpbnQgdGhlIHNjcm9sbGJhci4KKyAgICAgICAgVGhpcyBmaXhlcyB0aGUgcGFpbnRp bmcgaW4gUUdyYXBoaWNzdmlldywgd2hlbiB0aGUgcGFyZW50IG9mIHRoZSB3aWRnZXQgdGhhdCAK KyAgICAgICAgcGFpbnRzIHRoZSBRV2ViUGFnZSBoYXMgdGhlIFFHcmFwaGljc0l0ZW06Okl0ZW1D bGlwc0NoaWxkcmVuVG9TaGFwZSBzZXQgYW5kCisgICAgICAgIGEgcGllY2Ugb2YgdGhlIHNjcm9s bGJhciBzaG91bGQgYmUgY2lwcGVkIGF3YXkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTMwMzY2CisKKyAgICAgICAgTm8gbmV3IHRlc3RzLgorCisgICAg ICAgICogcGxhdGZvcm0vcXQvU2Nyb2xsYmFyVGhlbWVRdC5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpTY3JvbGxiYXJUaGVtZVF0OjpwYWludCk6CisKIDIwMDktMTItMTQgIEFsZXhleSBQcm9za3Vy eWFrb3YgIDxhcEBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGF2ZSBIeWF0dC4K SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vcXQvU2Nyb2xsYmFyVGhlbWVRdC5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9xdC9TY3JvbGxiYXJUaGVtZVF0LmNwcAkocmV2aXNpb24g NTE4NjUpCisrKyBXZWJDb3JlL3BsYXRmb3JtL3F0L1Njcm9sbGJhclRoZW1lUXQuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0xNDcsNyArMTQ3LDcgQEAKICAgICBwLnBhaW50ZXItPnNhdmUoKTsKICAg ICBRU3R5bGVPcHRpb25TbGlkZXIqIG9wdCA9IHN0eWxlT3B0aW9uU2xpZGVyKHNjcm9sbGJhciwg cC53aWRnZXQpOwogCi0gICAgcC5wYWludGVyLT5zZXRDbGlwUmVjdChvcHQtPnJlY3QuaW50ZXJz ZWN0ZWQoZGFtYWdlUmVjdCkpOworICAgIHAucGFpbnRlci0+c2V0Q2xpcFJlY3Qob3B0LT5yZWN0 LmludGVyc2VjdGVkKGRhbWFnZVJlY3QpLCBRdDo6SW50ZXJzZWN0Q2xpcCk7CiAKICNpZmRlZiBR X1dTX01BQwogICAgIHAuZHJhd0NvbXBsZXhDb250cm9sKFFTdHlsZTo6Q0NfU2Nyb2xsQmFyLCAq b3B0KTsK