Summary: | if the QWebPage is rendered by a QGraphicsWidget child of another widget with ItemClipsChildrenToShape the scrollbar is not clipped | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Marco Martin <notmart> | ||||||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Minor | CC: | commit-queue, eric, hausmann, kenneth, notmart | ||||||||
Priority: | P2 | Keywords: | Qt | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Marco Martin
2009-10-14 13:24:41 PDT
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 > 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); 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. |