Copy/ Paste after Select All is broken

VERIFIED FIXED in Firefox 68

Status

()

defect
P1
normal
VERIFIED FIXED
3 months ago
27 days ago

People

(Reporter: csheany, Assigned: m_kato)

Tracking

(Regression, {regression})

Firefox 68
Firefox 68
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox66 unaffected, firefox67 unaffected, firefox68 fixed)

Details

(Whiteboard: [triagemonth-2019-03] [bcs:p2])

Attachments

(4 attachments)

Reporter

Description

3 months ago

User Agent: Mozilla/5.0 (Android 7.1.1; Tablet; rv:68.0) Gecko/68.0 Firefox/68.0

Steps to reproduce:

  1. Open this bug
  2. View Page Source
  3. Select all
  4. Attach File

Actual results:

Text is not pasted

Expected results:

Text is pasted

Reporter

Comment 1

3 months ago

3.5 Copy :)

Hi :csheany,

Would you please elaborate the 4th step. What do you mean by 'Attach File'?

In my "Firefox Nightly for Android", I tried the following step:-

  • Open Firefox Nightly for Android 68.0a1

  • Visit this bug link

  • View Page Source

  • Select all the 'Page Source' elements and copy

  • And then I tried paste the elements in somewhere else

Actual Result : The elements of 'Page Source' are actually not copied successfully

Is this the problem you are trying to say?

Please, let us know so that we can move ahead!

Flags: needinfo?(csheany)
Whiteboard: [triagemonth-2019-03]
Reporter

Comment 3

3 months ago

Thank you for your response.

Yes, that is the problem.

Flags: needinfo?(csheany)
Reporter

Comment 4

3 months ago
Assignee

Updated

3 months ago
Component: General → Text Selection

Thanks for the report!
Tested on the latest Nightly build and I was able to reproduce the issue following the steps from description and comment 2.

Flags: needinfo?(csheany)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(csheany)
OS: Unspecified → Android
Hardware: Unspecified → ARM
Assignee

Comment 6

3 months ago

nsHTMLCopyEncoder::GetPromotedPoint throws error, so copy is failed. I will investigate this why this failure is only mobile.

Assignee: nobody → m_kato
Assignee

Comment 7

3 months ago

Before landing bug 676268, we don't copy HTML source, but after landing it, we try to copy HTML source.

And Fennec's action bar has a mistake to use selection all. Firefox/Desktop uses cmd_selectAll, but Fennec doesn't use it (cmd_selectAll isn't same as nsISelectionController.selectAll).

Assignee

Comment 8

2 months ago

After bug 676268 is landed, Gecko/Android supports text/html mime type on
clipboard. But copy command is sometimes failed after select all is executed.

nsISelectionContoller.selectAll is different of cmd_selectAll.
Since cmd_selectAll that is used on Firefox desktop doesn't select root
element, copy command always works well. So we should use it like desktop
browser on Fennec.

Also, GV already uses cmd_selectAll on action bar, so this is Fennec only.

Comment 9

2 months ago
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/6abaf5f96b42
Use cmd_selectAll instead of nsISelectionController.selectAll on action bar r=geckoview-reviewers,esawin

Comment 10

2 months ago
bugherder
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
No longer blocks: 676268
Regressed by: 676268

Comment 11

2 months ago

Hello!

I tested this following the steps from Comment 2, on the latest version of Nightly 68.0a1 (2019-04-10) with Samsung Galaxy Note 8 (Android 9), Nokia 6 (Android 7.1.1) and I can reproduce this issue. Moreover the browser is closing when tapping on the "Copy" option.

Video: https://drive.google.com/file/d/1NW9KxSk7ykIw0RsN8KGLisr1vzPhiVLG/view

Note:

  • On Nexus 6P (Android 8.1.0) the browser is not closing when tapping on the "Copy" option, but the issue is still reproducible.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter

Comment 12

2 months ago

Do you have any idea why this might be failing?

Flags: needinfo?(m_kato)
Assignee

Comment 13

2 months ago

(In reply to csheany from comment #12)

Do you have any idea why this might be failing?

When I test this, it work well. I guess that content is bigger than I test this issue, then TransactionTooLargeException is thrown. Although This is another issue, it is OK to investigate this on this issue.

Flags: needinfo?(m_kato)
Assignee

Comment 14

2 months ago

Even if I don't land bug 676268, I think that this will occurs with too large text.

Reporter

Comment 15

2 months ago

It seemed to be working initially.

Have there been additional patches that could have had an impact?

Assignee

Comment 16

2 months ago

(In reply to csheany from comment #15)

It seemed to be working initially.

Have there been additional patches that could have had an impact?

Yeah, I am working additional fix.

Assignee

Comment 18

2 months ago

When setting large clipboard data, it may cause TransactionTooLargeException
in binder IPC. So we have to handle RuntimeException.

Whiteboard: [triagemonth-2019-03] → [triagemonth-2019-03] [bcs:p2]

Comment 19

2 months ago
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/53793d2ea3ec
Handle RuntimeException when setting clipboard data r=geckoview-reviewers,snorp

Comment 20

2 months ago
bugherder
Status: REOPENED → RESOLVED
Closed: 2 months ago2 months ago
Resolution: --- → FIXED
Reporter

Comment 21

2 months ago

I'm getting a "Can't copy. Text too long" message.

I imagine that is a side effect of the patch but isn't ideal.

When first landed the functionality was in place.

Can someone help track down the "regression"?

I am still investigating on my end.

(In reply to csheany from comment #21)

I'm getting a "Can't copy. Text too long" message.

What is the exact wording of the error message? Does it look like an Android system message or a Fennec message?

Makoto's patch catches an Android RuntimeException and returns false. Perhaps the patch should pretend the copy succeeds and always return true? Perhaps returning false is triggering an error message in Gecko or Android.

Reporter

Comment 23

2 months ago
Reporter

Comment 24

2 months ago

If copying doesn't succeed the user should know.

I'm trying to figure out what made it seeem like the first patch never existed.

I'm surprised it broke again in less than a week.

Maybe "Text Copied To Clipboard" shouldn't be displayed?

Assignee

Comment 25

2 months ago

(In reply to csheany from comment #21)

I'm getting a "Can't copy. Text too long" message.

When copying HTML, we try to copy HTML format. If failed, we copy as plain text. So this message may be outputted to logcat when HTML copy is failed. But since we try to text after it, so text is stored to clipboard.

This is Android OS's clipboard limitation that is around 1MB due to binder transaction.

So before landing bug 676268, coping large text will be failed and Fennec may crash due to unhandled exception. After landing this, Fennec doesn't crash at this situation.

Tested on the latest Nightly build and the issue is not fixed. The browser is not closing but Paste & Go doesn't work.
The issue is not reproducible when we use just "paste".

Video: https://drive.google.com/file/d/1e0yEcwQRmRTSIrlOWa2zTVOW_fsp9Tuj/view?usp=sharing.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter

Comment 27

2 months ago

Makoto, thank you for the explanation.

Reporter

Comment 28

2 months ago

Not again! :)

I am unable to play the video.

What are you exlecting to Paste & Go?

(In reply to csheany from comment #28)

Not again! :)

I am unable to play the video.

What are you exlecting to Paste & Go?

The video is available for everyone with the link. For Paste & Go, I used a recommended by Pocket article.

Reporter

Comment 30

2 months ago

I had to download it but...

Would that be more of an issue with Paste & Go?

This was about it not being copied to the clipboard in the first place.

Assignee

Comment 31

2 months ago

(In reply to Sorina Florean [:sflorean] from comment #26)

Tested on the latest Nightly build and the issue is not fixed. The browser is not closing but Paste & Go doesn't work.
The issue is not reproducible when we use just "paste".

Video: https://drive.google.com/file/d/1e0yEcwQRmRTSIrlOWa2zTVOW_fsp9Tuj/view?usp=sharing.

Could you file new bug instead of reopen this? This seems to be Paste and Go implementation issue of address bar.

Assignee

Updated

2 months ago
Flags: needinfo?(sorina.florean)
Assignee

Comment 32

2 months ago

(Although I add to support text/html, address bar seems to allow formatted text. address bar shouldn't allow it then it only supports plain text that is included in text/html clipdata. So this is address bar issue.

Filed bug 1546890 for Paste&Go option. I will close this and mark as verified since Paste option is working.

Status: REOPENED → RESOLVED
Closed: 2 months ago2 months ago
Flags: needinfo?(sorina.florean)
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.