Revert changes from #863 #880
Conversation
iloveeclipse
requested review from
sravanlakkimsetti and
akurtakov
as code owners
|
@mickaelistria, @akurtakov : FYI. |
There was a problem hiding this comment.
I'm against reverting this patch. This has proven useful as it allows to copy paste between Eclipse and KDE apps on Wayland, which was impossible so far; so it's a great improvement.
If you can demonstrate it actually causes a regression on X11, I'd be OK to revert, but reverting a bugfix "just in case" is not the best way to move ahead.
The change was added too late (during M3 stabilization week)
Alex and I have approved it and tested it and assumed it's viable for M3 and important enough to not delay it further.
The fact that it was merged in stabilization week per se is not a good argument for a revert.
The change was not properly reviewed / tested.
We tested it, and while the code could have been better, it was evaluated as good enough.
I hope I can try #879 soon and it works as well so we can just merge it and close this chapter.
Put breakpoint into |
X11 port is proven to be stable, and Wayland support is proven to be not nice in many different areas. So honestly, I don't see why do we are rushing to have this patch in next release if we can get it (tested / fixed) in 4.31 in 3 months. On the other side, I don't want to put in danger many X11 users that wait for a stable 4.30 release with a patch that affects very basic functionality and is not thoroughly tested during release cycle. To avoid the revert, either #879 should be confirmed to work under Wayland, or at least a guard should be added around the questionable code in |
|
@mickaelistria |
There was a problem hiding this comment.
@mickaelistria
I have now tested this PR a second time and while TextTransfer copy past fully works as expected
RTFTransfer and HtmlTransfer
do not provide an uft-8 variant as I think the PR this is supposed to replace does.
Rerunning the exercise of testing the R_29 backport on this simplefied patch gives no meaning as the former PR was initiallw developed on R_29_Maintenance and needed the 3 added lines in ClipboardProxy.java
I would like to put this albeit much more simple and elegant solution on halt until after
2023_R
|
@the-snowwhite That still doesn't answer my question on #879 : do you think we should merge #879 now or not? |
|
Closing in favour of #879 |
Revert changes from #863 made to fix #851 for following reasons:
So to not affect 4.30 stability, I prefer to revert the change completely if the alternative proposal in #879 that fixes points above doesn't work on Wayland.