1815969 - If I copypaste from Gnome image viewer into bugzilla attachment page, I get a preview of an image but the content-type remains text/plain (which produces a textual attachment with just the local file path)

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Holbert [:dholbert]]

Filing a bug for an issue that I just encountered while posting attachments on bug 1815966 (see my obsoleted attachments there)

STR (on Ubuntu 22.04):

1. Open an image on your system by double-clicking in the (default) nautilus file-listing app.
2. Right-click the image (in the image viewer) and select "Copy"
3. Go to the Bugzilla add-attachment page, and focus the attachment textarea, and do Ctrl+V to paste.
   (Notice that you see a preview of the actual image)
4. Click through the form to submit the bugzilla attachment.

ACTUAL RESULTS:
Even though Bugzilla previewed the attachment as an image, it ends up being attached as a text file, with the contents being the image's local file path, like e.g. mtp://Google_Pixel_6a_29051JEGR08152/Internal%20shared%20storage/Pictures/Screenshots/Screenshot_20230209-113328.png in my case (attachment 9316867 [details] from bug 1815966).

EXPECTED RESULTS:
If Bugzilla previews the attachment as an image, it should attach it as an image.

(In my case I'm opening an image that lives on my Android phone, which is mounted at an mpt path. That's not a necessary part of the bug STR, though; it happens for images on my local hard drive as well.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: test attachment

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screencast of me creating the test attachment

Here's a screencast of me creating the previously-attached "test attachment" (which ends up as a plain text file)

(Disregard the "Restart Nightly" UI at the end; that's because I have a pending partially-installed update and can't create new content processes. That was just bad luck during me performing the STR on camera, & isn't related to this bug.)

Comment 3

[Daniel Holbert [:dholbert]]

As you can see in the screencast, the issue seems to be that the content-type doesn't get populated properly.

If I perform the STR with e.g. a copied-to-clipboard screenshot (from Firefox Screenshots, for example) or with the Firefox right-click "copy image" UI, then that content-type gets automatically updated to show PNG Image (image/png). For some reason, the right-click "Copy" UI from Gnome's image-viewer causes that content-type to get auto-populated to "text/plain", despite the fact that Bugzilla properly previews the image in its add-attachment UI.

Comment 4

[Daniel Holbert [:dholbert]]

Aha, this seems to be a Firefox-specific issue, actually!

If I perform the STR in Chrome (Blink) or Epiphany (WebKit), the content-type field does get auto-populated properly.

I suspect this is an issue with how we interpret the clipboard contents, so it probably belongs in the DOM: Copy & Paste component

Comment 5

[Andreas Farre [:farre]]

Edgar, you know the most about Copy & Paste currently. Do you have an opinion on priority and severity?

Comment 6

[Edgar Chen [:edgar]]

I poke this issue a bit, this doesn't look like a copy & paste issue, as the DataTransfer do contain the proper image data, and I also saw the Content Type being selected to image/jpeg correctly while page handle the paste event, but somehow the content type changes back to text/plain later, not sure what cause the page to switch type back.

Comment 7

[Edgar Chen [:edgar]]

Okay, I guess I know what happens here. When user do Ctrl+V to paste on textarea, except the image data, browser also tries to set the text data to the editor. And after handling image data from the paste event, BMO move the focus to the Description input field in paste event handler, and the text data is still pasted to textarea, but other browsers paste the text data to the input field instead.

Comment 8

[Edgar Chen [:edgar]]

Attachment: test.html

Here is the test page to show the difference

STR:

1. Copy string into clipboard, e.g. select some text and press Ctrl+C
2. Click textarea (green one)
3. Do Ctrl+V to paste string

Gecko:
Text is pasted to the textarea.

Chrome/Webkit:
Text is pasted to the input field.

Comment 9

[Edgar Chen [:edgar]]

I guess this is handled in Editor code and is a long-existing behavior, masayuki, do you have any idea about this? Thanks!

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, the reason is, the event handler does not call preventDefault of paste event nor beforeinput event. Therefore, the editor keeps handling the event. I believe that the behavior must be correct for conforming to the design of DOM events. However, indeed, if the other browsers work differently, we need to align it to them. There is a concern about security/privacy... If new focused element is in different origin's document, it may cause leading sensitive content like passwords. Perhaps, we should redirect the input only when new editor is in same document, otherwise, perhaps, we should just discard the input. I'll check whether the behavior change actually fixes this bug or not.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sorry for the long delay. Yeah, redirecting paste command handling to new focused element works. I'll try to fix this, but this requires a lot of changes since there are a lot of paste command handlers...

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1815969 - part 1: Redesign `EditorBase::FireClipboardEvent` r=m_kato!

It does not handle critical cases properly, and it uses an out-param.
We can rewrite it with Result. However, unfortunately,
nsCopySupport::FireClipboardEvent does not return error. Therefore,
the root callers still need to treat the error cases as "canceled".

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1815969 - part 2: Make `PasteAsAction` and `PasteAsQuotationAsAction` stop taking `bool` argument r=m_kato!

Depends on D176737

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1815969 - part 3: Redesign `EditorBase::PasteAsAction` and `EditorBase::PasteAsQuotationAsAction` r=m_kato!

This patch makes EditorBase implement them as non-virtual methods and
implement their first part. Then, they call new virtual methods to handle
paste and paste-as-quotation.

With this change, TextEditor starts to dispatch paste event when
paste-as-quotation. The new test checks it.

Depends on D176738

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1815969 - part 4: Redesign `PasteTransferableAsAction` as same as the methods touched by the previous patch r=m_kato!

Depends on D176739

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1815969 - part 5: Make editors handle pasting something in new focused editor if a `paste` event listener moves focus r=m_kato!

If a paste event listener moves focus, Chrome makes new editor keep handling
the pasting. For the compatibility, we should follow it unless the new focused
element is in different document because user should allow to paste it
explicitly.

On the other hand, this just stops handling "cut" in same situation because
handling it requires to update clipboard without user's activation. Therefore,
the clipboard content and/or the new editor content may be lost from the users
point of view.

Note that nsContentUtils::GetActiveEditor may return HTMLEditor instance
when focused element does not have TextEditor even when non-editable element
has focus. Therefore, if it returns an HTMLEditor, we need to check whether
it's active in the DOM window with a call of HTMEditor::IsActiveInDOMWindow.

Depends on D176740

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a42837f38b02
part 1: Redesign `EditorBase::FireClipboardEvent` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/644cbb12a064
part 2: Make `PasteAsAction` and `PasteAsQuotationAsAction` stop taking `bool` argument r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d36c58bcd9d7
part 3: Redesign `EditorBase::PasteAsAction` and `EditorBase::PasteAsQuotationAsAction` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/5cbc7a690417
part 4: Redesign `PasteTransferableAsAction` as same as the methods touched by the previous patch r=m_kato
https://hg.mozilla.org/integration/autoland/rev/1a47a7baf147
part 5: Make editors handle pasting something in new focused editor if a `paste` event listener moves focus r=m_kato

Comment 18

[Serban Stanca [:SerbanS]]

Backed out for causing build bustages in EditorBase.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/editor/libeditor/EditorBase.cpp:1769:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type]

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cee2b730ce01
part 1: Redesign `EditorBase::FireClipboardEvent` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f0b845ef7e0e
part 2: Make `PasteAsAction` and `PasteAsQuotationAsAction` stop taking `bool` argument r=m_kato
https://hg.mozilla.org/integration/autoland/rev/12d103c98be8
part 3: Redesign `EditorBase::PasteAsAction` and `EditorBase::PasteAsQuotationAsAction` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/aee951a66c25
part 4: Redesign `PasteTransferableAsAction` as same as the methods touched by the previous patch r=m_kato
https://hg.mozilla.org/integration/autoland/rev/55c1f2ce765b
part 5: Make editors handle pasting something in new focused editor if a `paste` event listener moves focus r=m_kato

Comment 20

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/cee2b730ce01
https://hg.mozilla.org/mozilla-central/rev/f0b845ef7e0e
https://hg.mozilla.org/mozilla-central/rev/12d103c98be8
https://hg.mozilla.org/mozilla-central/rev/aee951a66c25
https://hg.mozilla.org/mozilla-central/rev/55c1f2ce765b

Comment 21

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-05-08 Nightly build on Ubuntu 22.
Verified as fixed on Firefox 115.0b6(build ID: 20230615175802) and Nightly 116.0a1(build ID: 20230616094829) on Ubuntu 22, macOS 12, Windows 10.