Opened 4 years ago

Closed 4 years ago

#13187 closed Bug (fixed)

Paste check doesn't work.

Reported by: Artur Delura Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.5.0
Component: General Version: 4.5.0 Beta
Keywords: Cc: IRINAURU@…, giorgio, satya_minnekanti@…, chrisgui@…

Description

  1. Open new sample in presets.
  2. Select some text external/internal.
  3. Paste into editor.

I doesn't paste content.

It's a regression, it worked in 4.4.7.

Change History (12)

comment:1 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.0

comment:2 Changed 4 years ago by Jakub Ś

Status: newconfirmed

comment:3 Changed 4 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:4 Changed 4 years ago by Piotr Jasiun

Status: assignedreview

Unfortunately clipboard API does not work fine on the Android Chrome and we need to use paste bin there.

Because there are differences between mobile and desktop Chrome I changed CKEDITOR.env, so now Android Chrome is marked as mobile and introduced CKEDITOR.env.android flag. After these changes this ticket should be closed: #11725.

Changes in t/13187.

comment:5 Changed 4 years ago by Piotrek Koszuliński

Status: reviewreview_failed

On Chrome running in desktop mode this check does not work, because it is recognised as a desktop browser, but has the same, limited API.

comment:6 Changed 4 years ago by Piotr Jasiun

So... Unfortunately on the android in the desktop mode userAgent looks like desktop, what we could expected (Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.96 Safari/537.36), but the API is still mobile, so Clipboard API still does not work, what we could expected too.

The situation is even worst because I can not detect feature by setting and getting custom data, because it is not possible to setData on paste on Chrome in general.

The solution would be to change the way we check if we should use paste bin or clipboardData. We could use clipboardData if it not empty and the browser support HTML in it.

I pushed such solution to the t/13187b and checked all browsers:

  • on IE and Safari we need to use pastebin, because HTML data are not available other way during the external drop (only text data are available via clipboardData),
  • on Firefox the bug mentioned in http://dev.ckeditor.com/ticket/11461#comment:8 seems to be fixed and we should be able to rely on the clipboardData; I did not believe, but I tested it on Linux and Windows and the same data are available in clipboardData and pasteBin in both cases; on Linux it is not possible to get file any way, on Windows it works fine using clipboard data,
  • Chrome works fine using the new method on Linux, Windows and Android.

comment:7 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

I realized that after that change internal/cross editor drag and drop on Safari starts using pasteBin, and they should not, so I have added one more condition to check the transfer type.

To be honest I do not like this solution because it touch a lot strange cases and we can easily forget about something. On the other hand it for not only Chrome@Android-inDesktopMode case, but also update Firefox behavior after they fix the issue.

At this stage it could be reviewed.

comment:8 Changed 4 years ago by Piotrek Koszuliński

http://dev.ckeditor.com/ticket/13264#comment:5 this comment clearly proves that the current check doesn't make sense.

comment:9 Changed 4 years ago by Piotrek Koszuliński

Cc: IRINAURU@… giorgio satya_minnekanti@… chrisgui@… added
Status: reviewassigned

Closed #13264 as a DUP of this ticket. See http://dev.ckeditor.com/ticket/13264#comment:6 for what is expected from this ticket.

comment:10 Changed 4 years ago by Piotr Jasiun

Status: assignedreview

I created a manual test to check if everything is fine. We need to test how it works on all browsers. Changes in t/13187b .

Last edited 4 years ago by Piotr Jasiun (previous) (diff)

comment:11 Changed 4 years ago by Piotrek Koszuliński

I rebased and pushed few commits to branch:t/13187b.

comment:12 Changed 4 years ago by Piotr Jasiun

Resolution: fixed
Status: reviewclosed
Summary: Paste doesn't work on android chromePaste check doesn't work.

Deeply tested and closed with git:0e2ae53.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy