[PATCH] Check size when generating FrameSvg background
(reviewboard seems to be down today, so sending this the old-fashioned way)
Hi,
Stumbled upon a bad_alloc exception in Plasma today: after some
debugging I found out that sometimes
FrameSvgPrivate::generateFrameBackground() tried to generate a
background of size 16777215 x 13. 16777215 is QWIDGETSIZE_MAX.
It seems the buggy size comes from the way the System Monitor applet
uses Plasma::Frame, but I thought adding a generic guard in
generateFrameBackground() would be useful, hence the attached patch.
Actually I am thinking it may make sense to set stricter limits, for
example ensuring each dimension is less than say 1 pixels.
What do you think about this?
Aurélien
diff --git a/plasma/framesvg.cpp b/plasma/framesvg.cpp
index 981e8fa..611acc0 100644
--- a/plasma/framesvg.cpp
+++ b/plasma/framesvg.cpp
@@ -27,6 +27,7 @@
#include QRegion
#include QTimer
#include QCryptographicHash
+#include QWidget
#include kdebug.h
@@ -490,6 +491,10 @@ void FrameSvgPrivate::generateFrameBackground(FrameData *frame)
kWarning() Invalid frame size size;
return;
}
+if (size.width() == QWIDGETSIZE_MAX || size.height() == QWIDGETSIZE_MAX) {
+kWarning() Not generating frame background for a size whose width or height is QWIDGETSIZE_MAX size;
+return;
+}
const int contentWidth = size.width() - frame-leftWidth - frame-rightWidth;
const int contentHeight = size.height() - frame-topHeight - frame-bottomHeight;
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: [PATCH] Check size when generating FrameSvg background
On Tuesday 22 December 2009, Aurélien Gâteau wrote: (reviewboard seems to be down today, so sending this the old-fashioned way) Hi, Stumbled upon a bad_alloc exception in Plasma today: after some debugging I found out that sometimes FrameSvgPrivate::generateFrameBackground() tried to generate a background of size 16777215 x 13. 16777215 is QWIDGETSIZE_MAX. It seems the buggy size comes from the way the System Monitor applet uses Plasma::Frame, but I thought adding a generic guard in generateFrameBackground() would be useful, hence the attached patch. Actually I am thinking it may make sense to set stricter limits, for example ensuring each dimension is less than say 1 pixels. What do you think about this? Aurélien well, with this patch it would still break with qwidgetsizemax-1 :p so yeah a stricter limit would probably make sense, perhaps as a const int in framesvgprivate -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: [PATCH] Check size when generating FrameSvg background
Marco Martin wrote:
On Tuesday 22 December 2009, Aurélien Gâteau wrote:
(reviewboard seems to be down today, so sending this the old-fashioned way)
Hi,
Stumbled upon a bad_alloc exception in Plasma today: after some
debugging I found out that sometimes
FrameSvgPrivate::generateFrameBackground() tried to generate a
background of size 16777215 x 13. 16777215 is QWIDGETSIZE_MAX.
It seems the buggy size comes from the way the System Monitor applet
uses Plasma::Frame, but I thought adding a generic guard in
generateFrameBackground() would be useful, hence the attached patch.
Actually I am thinking it may make sense to set stricter limits, for
example ensuring each dimension is less than say 1 pixels.
What do you think about this?
Aurélien
well, with this patch it would still break with qwidgetsizemax-1 :p
so yeah a stricter limit would probably make sense, perhaps as a const int in
framesvgprivate
Agreed, attached is an updated patch.
Aurélien
diff --git a/plasma/framesvg.cpp b/plasma/framesvg.cpp
index 981e8fa..5871a07 100644
--- a/plasma/framesvg.cpp
+++ b/plasma/framesvg.cpp
@@ -36,6 +36,10 @@
namespace Plasma
{
+// Any attempt to generate a frame whose width or height is larger than this
+// will be rejected
+static const int MAX_FRAME_SIZE = 10;
+
FrameSvg::FrameSvg(QObject *parent)
: Svg(parent),
d(new FrameSvgPrivate(this))
@@ -490,6 +494,10 @@ void FrameSvgPrivate::generateFrameBackground(FrameData *frame)
kWarning() Invalid frame size size;
return;
}
+if (size.width() = MAX_FRAME_SIZE || size.height() = MAX_FRAME_SIZE) {
+kWarning() Not generating frame background for a size whose width or height is more than MAX_FRAME_SIZE size;
+return;
+}
const int contentWidth = size.width() - frame-leftWidth - frame-rightWidth;
const int contentHeight = size.height() - frame-topHeight - frame-bottomHeight;
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: [PATCH] Check size when generating FrameSvg background
On Tuesday 22 December 2009, Aurélien Gâteau wrote: Marco Martin wrote: On Tuesday 22 December 2009, Aurélien Gâteau wrote: (reviewboard seems to be down today, so sending this the old-fashioned way) Hi, Stumbled upon a bad_alloc exception in Plasma today: after some debugging I found out that sometimes FrameSvgPrivate::generateFrameBackground() tried to generate a background of size 16777215 x 13. 16777215 is QWIDGETSIZE_MAX. It seems the buggy size comes from the way the System Monitor applet uses Plasma::Frame, but I thought adding a generic guard in generateFrameBackground() would be useful, hence the attached patch. Actually I am thinking it may make sense to set stricter limits, for example ensuring each dimension is less than say 1 pixels. What do you think about this? Aurélien well, with this patch it would still break with qwidgetsizemax-1 :p so yeah a stricter limit would probably make sense, perhaps as a const int in framesvgprivate Agreed, attached is an updated patch. Aurélien good to go i think -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Bug 198661 (Option to disable scroll wheel on taskbar)
On December 19, 2009, Gábor Lehel wrote: decided to put a giant clock or pager along the middle of their bottom panel they would have similar issues, but who does that? it happens less frequently with smaller targets, but it does happen. and yes, we do get reports about it. so no, i'm not interested in patches that fix it in one place or even multiple patches that fix it in multiple places with independent settings. one global option should do. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: webkit scriptengine multiple plasmoids
On December 20, 2009, Petri Damstén wrote: On Friday 18 December 2009 00:17:19 Marco Martin wrote: On Thursday 17 December 2009, Petri Damstén wrote: Hi, I'm trying to fix this bug: http://www.kde- look.org/content/show.php/Scripted+Image?content=91749 (Error having 2 of them). Seems that if there is two (or more) same webkit plasmoids on desktop only one of them loads properly on plasma start. When same plasmoid B load is started before A load is finished javascript functions don't get called. This happens only when plasmoids are same e.g. two scripted-image plasmoids. I made attached patch to load plasmoids with the same plugin name in queue and it seems to solve this but I'm not sure if this right way to do it? Petri seems an hacky workaround, but i can'r really think about a possible real cause of this... could be a bug in webkit itself, maybe has some internal sigleton class that gets in an inconsistent state when two pages load the same url in the same moment? seems terribly unlikely but... I can reproduce this bug with a small Qt only app (http://aryhma.oy.cx/damu/webkit-test.tar.gz). It seems that if the same javascript file is included in multiple html files that are loaded at the same time then loadFinished signal is emitted too early. Should this have a workaround in webkit script engine until this is fixed in Qt/webkit? if it has been reported against Qt/Webkit, then yes, we can have a (documented) work around in the code that we can remove when Qt/Webkit is fixed. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Solid - places runners conflict
On December 18, 2009, Jacopo De Simoi wrote: Dear plasmadevs, I just noticed that we have some feature conflict among the runners; in fact the places runner shows solid devices as well and triggers dolphin on them upon selection. Since this feature is included in solid runner, which is specialized to treat solid devices in a fairly complete way I propose to disable the feature from the places runner. does the solid runner match custom items added to the places model? -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: [PATCH] Check size when generating FrameSvg background
On December 22, 2009, Aurélien Gâteau wrote: good to go i think OK, it's in. nice catch :) -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Solid - places runners conflict
On Tuesday 22 December 2009 22:41:01 Aaron J. Seigo wrote: On December 18, 2009, Jacopo De Simoi wrote: Dear plasmadevs, I just noticed that we have some feature conflict among the runners; in fact the places runner shows solid devices as well and triggers dolphin on them upon selection. Since this feature is included in solid runner, which is specialized to treat solid devices in a fairly complete way I propose to disable the feature from the places runner. does the solid runner match custom items added to the places model? Nope, just devices. I don't intend to disable the places runner itself, but just the matches referring to devices. --J ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: KWin highlight window effect enabled by default?
Am Dienstag 22 Dezember 2009 22:42:55 schrieb Aaron J. Seigo: On December 20, 2009, Lucas Murray wrote: Just to summarise: I consider that enabling the KWin highlight window effect by default is the wrong choice and that the change should be reverted before the next KDE SC release. fwiw, i whole-heartedly agree with everything Lucas wrote. +1 from me. perhaps the person(s) who enabled it by default could help us understand the reasoning more? if there isn't a compelling reason there, i think Lucas is quite correct. I run the effect several days without noticiing any of the problems why it wasn't enabled in 4.3. Lucas noticed some problems and those were fixed (e.g. with multiple desktops) or should be fixed (e.g. the mentioned problem on fast movement). Personally I think this is an effect which adds lots of functionality and usability. For me it is very useful for example to just have a peak at a window, to read a few lines of text on a web page or to look at the opened documentation in Qt Assistant... Of course I'm an experienced user and have not done any usability studies on that subject ;-) Of course I understand the reasoning that it is one of Windows 7 main user interface points. But I think we have enough unique features and new improvements in 4.4 that we do not have to fear that users think we only copy from Windows and it should not be a reason to turn off useful features. With the same arguments we would have to turn off for example Present Windows as it's just a copy of MacOS. The highlight windows effect is not only used for the taskbar highlights, but I also turned it on in the classic non-composited tabbox to provide the same highlight of the current selected window as the boxswitch effect does. In fact I was thinking about switching off boxswitch by default in favor for the classic tabbox with the highlight effect as that one scales better for a larger number of windows (thumbnails are too small in boxswitch effect). So here we have the problem that turning on the effect is not side-effect free. If you want to use it for alt+tab you have to use it for taskbar as well as it is not configurable (and it's too late for 4.4). Personally I do not object to a default off although I do not have the same doubts as mentioned by Lucas. signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: Compile fix for geolocation dataengine with gpsd 2.90
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2443/ --- Review request for Plasma. Summary --- This patch makes it possible to compile the plasma geolocation dataengine with gpsd 2.90 and later versions. gpsd 2.90 is the version which is used in Debian sid. This patch is based on 27_geolocation_gpsd_2.90_compat.diff by Modestas Vainius which is used in the kdebase-workspace 4.3.4-1 package. It only adds conditional compilation based on gpsd API version, so that users with the old gpsd can continue to compile. Diffs - /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/geolocation/location_gps.cpp 1065302 Diff: http://reviewboard.kde.org/r/2443/diff Testing --- compile tested. Thanks, jmho ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: Compile fix for geolocation dataengine with gpsd 2.90
--- This is an automatically generated e-mail, which was bounced and now sent manually. To reply, visit: http://reviewboard.kde.org/r/2443/ --- Review request for Plasma. Summary --- This patch makes it possible to compile the plasma geolocation dataengine with gpsd 2.90 and later versions. gpsd 2.90 is the version which is used in Debian sid. This patch is based on 27_geolocation_gpsd_2.90_compat.diff by Modestas Vainius which is used in the kdebase-workspace 4.3.4-1 package. It only adds conditional compilation based on gpsd API version, so that users with the old gpsd can continue to compile. Diffs - /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/geolocation/location_gps.cpp 1065302 = Diff: http://reviewboard.kde.org/r/2443/diff Testing --- compile tested. Thanks, jmho ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Restoring plasma activity and Giving credits
Hi all, When using Search and Launch Containment is there a way to restore to Desktop Activity in Desktop Settings other than Alt+D, Alt+S ? Right-clicking in desktop doesn't show anything. In addition, the popup menu item's label Desktop Activity Setttings doesn't match the caption of opened windows (Desktop Settings). I think some stuff other than activitied can be set up there. A second question: I would like to give credits for a contribution in KDE Observatory, where should I include this ? Is there any credits section in .desktop file or can I insert more than one author ? Thanks in advance, Sandro ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Restoring plasma activity and Giving credits
Ok, creating a new mouse action solved the problem. Is this expected to not be enabled by default ? Sandro On Wed, Dec 23, 2009 at 1:32 AM, Sandro Andrade sandroandr...@kde.org wrote: Hi all, When using Search and Launch Containment is there a way to restore to Desktop Activity in Desktop Settings other than Alt+D, Alt+S ? Right-clicking in desktop doesn't show anything. In addition, the popup menu item's label Desktop Activity Setttings doesn't match the caption of opened windows (Desktop Settings). I think some stuff other than activitied can be set up there. A second question: I would like to give credits for a contribution in KDE Observatory, where should I include this ? Is there any credits section in .desktop file or can I insert more than one author ? Thanks in advance, Sandro ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: KWin highlight window effect enabled by default?
On Tue, Dec 22, 2009 at 11:07:00PM +0100, Martin Gräßlin wrote: The highlight windows effect is not only used for the taskbar highlights, but I also turned it on in the classic non-composited tabbox to provide the same highlight of the current selected window as the boxswitch effect does. In fact I was thinking about switching off boxswitch by default in favor for the classic tabbox with the highlight effect as that one scales better for a larger number of windows (thumbnails are too small in boxswitch effect). So here we have the problem that turning on the effect is not side-effect free. If you want to use it for alt+tab you have to use it for taskbar as well as it is not configurable (and it's too late for 4.4). This changes things somewhat as I didn't even know that feature existed. Since the window switcher cannot use the effect without it being enabled for the taskbar as well it's time to mention that Plasma should really have its own settings for enabling both highlight windows and taskbar thumbnails. Both effects are intended to be passive--as in they are always enabled and are used whenever an application requests it. As they can be used by multiple applications at the same time only having a single global setting doesn't really work that well. The only hurdle for this right now is string freeze so if a string exception cannot be gained (Since we're already past beta 2 I doubt one can be given) I guess the Plasma highlight effect setting will need to be a hidden option in KDE SC 4.4. -- Lucas Murray GPG Fingerprint: 0B88 499E 3F5B 1405 D952 258A AD90 B4F5 90B6 3534 pgpNdE1XJWvYU.pgp Description: PGP signature ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Restoring plasma activity and Giving credits
On December 22, 2009 17:54:06 Sandro Andrade wrote: Ok, creating a new mouse action solved the problem. Is this expected to not be enabled by default ? no. I've heard of a few people not getting any default mouse actions... but they work for *me*. I guess I'll have to investigate after my vacation :/ but you're supposed to always have a cashew... and I thought the SAL was a netbook-only thing. notmart, are people *supposed* to be able to choose it in the activity settings? -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Compile fix for geolocation dataengine with gpsd 2.90
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2443/#review3501 --- Ship it! i just love the api stability with free software projects. :/ looks good, please commit :) - Aaron On 2009-12-23 00:03:18, jmho wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2443/ --- (Updated 2009-12-23 00:03:18) Review request for Plasma. Summary --- This patch makes it possible to compile the plasma geolocation dataengine with gpsd 2.90 and later versions. gpsd 2.90 is the version which is used in Debian sid. This patch is based on 27_geolocation_gpsd_2.90_compat.diff by Modestas Vainius which is used in the kdebase-workspace 4.3.4-1 package. It only adds conditional compilation based on gpsd API version, so that users with the old gpsd can continue to compile. Diffs - /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/geolocation/location_gps.cpp 1065302 Diff: http://reviewboard.kde.org/r/2443/diff Testing --- compile tested. Thanks, jmho ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Solid - places runners conflict
On December 22, 2009, Jacopo De Simoi wrote: I don't intend to disable the places runner itself, but just the matches referring to devices. that's probably ok; my only concern would be where the user changes the name of a label of a device in the places model. then it wouldn't match in krunner, even though it would be visible in the places views (dolphin, etc). if we can cover that use case, such that the places runner only filters out devices that haven't been renamed, then we're ok. otherwise we'll need something more sophisticated in the runner manager, i fear, where it can filter out identical matches so if they are returned by multiple runners it only shows one entry. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Compile fix for geolocation dataengine with gpsd 2.90
Am Mittwoch, 23. Dezember 2009 06:46:15 schrieb Aaron Seigo: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2443/#review3501 --- Ship it! i just love the api stability with free software projects. :/ looks good, please commit :) - Aaron commited. Thanks for quick review. Jens-Michael ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Solid - places runners conflict
On Wednesday 23 December 2009 06:50:50 Aaron J. Seigo wrote: On December 22, 2009, Jacopo De Simoi wrote: I don't intend to disable the places runner itself, but just the matches referring to devices. that's probably ok; my only concern would be where the user changes the name of a label of a device in the places model. then it wouldn't match in krunner, I am not able to change the label of a device with dophin, but I'm using dolphin and kfile libs from 4.3, although I don't see any relevant change in websvn. Can anybody confirm this? ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
