Review Request 108808: Do not reset opacity if call for GetWindowProperty fails

[Martin Gräßlin]

> On Feb. 6, 2013, 4:29 p.m., Thomas Lübking wrote:
> > If the property does not exist or is of wrong type or contains junk, the reasonable return value would be 1.0, not 0.0 - agreed?
> > I mean, if the client withdraws the property and it would get read agin, the client would expect to be opaque as if it didn't support the property, yesno?
> > 
> > If the call gets us a BadWindow (likely the case except for below) it seems pretty much ok to return the cached value, but i don't think that's the case for other errors.
> > 
> > reg. colibri:
> > i don't see why Qt should withdraw the property before deleting the window.
> > Any chance that colibri attempts to set the opacity out of bounds (-1) so that Qt would fall back to withdrawing it or similar?

> If the property does not exist or is of wrong type or contains junk, the reasonable return value would be 1.0, not 0.0 - agreed?
yes

> If the call gets us a BadWindow (likely the case except for below) it seems pretty much ok to return the cached value, but i don't think that's the case for other errors.
actually it's fine for all the error cases to return the default value. What might need to be considered is the case of incorrect data.

> reg. colibri:
> i don't see why Qt should withdraw the property before deleting the window.
it doesn't delete it

I think I'll write a unit test tomorrow to see what's actually going on.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108808/#review26760
-----------------------------------------------------------


On Feb. 6, 2013, 3:38 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108808/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2013, 3:38 p.m.)
> 
> 
> Review request for kdelibs and kwin.
> 
> 
> Description
> -------
> 
> Do not reset opacity if call for GetWindowProperty fails
> 
> The opacity got unconditionally reset to the default value of fully
> opaque. In case the property has a value of 0 and the call to get the
> window property fails, the window would be shown again. This issue was
> spotted with colibri which faded out it's window and got a flash to
> fully opaque again.
> 
> I'm sorry that it's difficult to read this patch properly due to the
> existing mixing of tabs and whitespaces. The patch changes two things:
> * only updates the value if all checks succeed
> * initialise the data_ret variable to not cause a crash on free
> 
> BUG: 314427
> FIXED-IN: 4.10.1
> 
> 
> Diffs
> -----
> 
>   kdeui/windowmanagement/netwm.cpp cf28339f90dff1d17ed17842c7c11d8a9718a459 
> 
> Diff: http://git.reviewboard.kde.org/r/108808/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130206/0995b414/attachment.htm>