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, elias
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
T
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, al
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 KWayl
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: graes
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 th
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, pro
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 had
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 r
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.
Relevant
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 (!sele
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)
> d->createDataOffer(other->selection()
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 guarded
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
