graesslin abandoned this revision.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
To: graesslin, #kwin, #plasma, #frameworks
Cc: cfeck, anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot,
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,
davidedmundson added a comment.
I'm still against it.
Passing unexpectedly invalid pointers about all over the place and then
trying to guard them afterwards is an anti pattern that leads to future
problems.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
graesslin added a comment.
I'm still in favor of pushing this one too.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
To: graesslin, #kwin, #plasma, #frameworks
Cc: cfeck, anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot,
progwolff, lesliezhai,
cfeck added a comment.
https://phabricator.kde.org/D7316 has been committed, and the referenced bug
marked as fixed.
Reading above comments, this patch can/should be committed, too. Please check
if this is still true, and either approve this patch, or discard it.
REPOSITORY
R127
graesslin added a comment.
> If your patch works, then mine would too. It's checking the exact same
pointer, just one frame earlier.
But then it's an easy thing: let's do both. Your patch and mine.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
To:
davidedmundson added a comment.
> of course we do that :-)
We're not.
Kwin does not check if a client has a selection.
It merely checks if the client has an interface from a selection can be
fetched, it does not check that device actually has a valid selection.
> you think
anthonyfieroni added a comment.
But what is point to have a DataOfferInterface without valid source?
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
To: graesslin, #kwin, #plasma, #frameworks
Cc: anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot,
graesslin added a comment.
In https://phabricator.kde.org/D7078#135036, @davidedmundson wrote:
> > So from KWayland Server side we have a selection
>
> from KWayland server side we have a Seat:selection, we don't check if that
has a DDI::selection
>
>
>
> If a client
davidedmundson added a comment.
> So from KWayland Server side we have a selection
from KWayland server side we have a Seat:selection, we don't check if that
has a DDI::selection
If a client had called wl_set_selection(dataDevice, nulltptr) the server
would still have a
graesslin added a comment.
> the send_selection spec says we should be passing a null resource if we
have no selection, whereas this version creates a data offer with nothing in it.
But we have a selection, that's the point. We just don't have a datasource on
the selection.
davidedmundson added a comment.
I don't think we did go in there because we have a selection, otherwise
other->selection() wouldn't be null.
If we want DDI::sendSelection(DDI *other) to always keep the client in sync
we should do:
auto selection = other->selection();
if
graesslin added a comment.
In https://phabricator.kde.org/D7078#134111, @davidedmundson wrote:
> Isn't the real bug from that trace here:
>
> datadevice_interface.cpp:204
> void DataDeviceInterface::sendSelection(DataDeviceInterface *other)
>
davidedmundson added a comment.
Isn't the real bug from that trace here:
datadevice_interface.cpp:204
void DataDeviceInterface::sendSelection(DataDeviceInterface *other)
d->createDataOffer(other->selection());
it's valid for other->selection() to be null, so this should be
graesslin added a comment.
ping! This is a crash fix.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7078
To: graesslin, #kwin, #plasma, #frameworks
Cc: plasma-devel, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, eliasp, sebas, apol,
graesslin created this revision.
Restricted Application added projects: Plasma on Wayland, Frameworks.
Restricted Application added a subscriber: plasma-devel.
REVISION SUMMARY
This crash got triggered in KWin when sending the selection to XWayland.
While the test handles condition of not yet
15 matches
Mail list logo