Closed Bug 1327798 Opened 7 years ago Closed 7 years ago

"Paste as plain text" Ctrl+Shift+V reports HTML in e.clipboardData.getData('text/html')

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: arni2033, Assigned: m_kato)

References

Details

Attachments

(2 files)

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
Read STR_1 and STR_2, but most likely you only need STR_3

>>>
STR_1:  (original)
1. Log in on https://mail.yandex.ru/ , start writing a new email
2. Type normal text "hello ". Type text "world", select that text, click button to make it bold.
3. Copy "world" to clipboard. Click between "hell" and "o", press Ctrl+Shift+V

AR:  Bold text "world" is pasted
ER:  "world" should be pasted as normal text. That's what shortcut Ctrl+Shift+V for.


STR_2:  (investigation: Yandex mail uses CKEditor)
1. Open this url, doubleclick "world", press Ctrl+C   data:text/html,<div contenteditable><b>world
2. Open http://ckeditor.com/demo
3. Click between "space" and "flight" in word "spaceflight", press Ctrl+Shift+V



STR_3:  (investigation: Yandex mail uses CKEditor)
1. Open url [1]
2. Doubleclick "world", press Ctrl+C to copy it to clipboard
3. Click between "hell" and "o" in word "hello", press Ctrl+Shift+V

AR:  e.clipboardData.getData('text/html') says I'm pasting some HTML
ER:  e.clipboardData.getData('text/html') should be empty, as if I pasted plain text "world",
     just like in GoogleChrome. Because Ctrl+Shift+V is shortcut called "paste as plain text"

> [1] data:text/html,<body onload="document.body.onpaste = function(e) {console.log(e.clipboardData.getData('text/html'));e.preventDefault();}"><div contenteditable>hello <b>world</b>
No longer blocks: 1277113
Component: Untriaged → DOM: Events
Product: Firefox → Core
Editor sounds a better fit... I guess?
Component: DOM: Events → Editor
Priority: -- → P3
See Also: → 1331752
Comment on attachment 8832779 [details] [diff] [review]
Part 1. PasteNoFormatting shouldn't set text/html to clipboard event on paste

When using no formatting paste (by Ctrl+Shift+V), Chrome returns text/plain only on clipboard object on paste event.  But Gecko passes both text/plain and text/html even if no formatting paste.

So, when web developer wants to use e.preventDefault() on paste event and insert custom text, they cannot detect whether user uses normal paste or no formatting paste.
Attachment #8832779 - Flags: review?(enndeakin)
Attachment #8832780 - Flags: review?(enndeakin)
Comment on attachment 8832779 [details] [diff] [review]
Part 1. PasteNoFormatting shouldn't set text/html to clipboard event on paste

> 
> bool
>-nsCopySupport::FireClipboardEvent(EventMessage aEventMessage,
>+nsCopySupport::FireClipboardEvent(EventMessage aOriginalEventMessage,

I think it would be clearer to leave this as is and use a local 'originalEventMessage' instead.

Otherwise this seems like a suitable way to handle this. Looks like Chrome does the same thing by stripping out non-text.
Attachment #8832779 - Flags: review?(enndeakin) → review+
Comment on attachment 8832780 [details] [diff] [review]
Part 2. Add test for no formatting

Test looks ok, but I suspect this test may fail on Linux intermittently, due to not waiting for the clipboard copy to occur.
Attachment #8832780 - Flags: review?(enndeakin) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac3511a43396
Part 1. PasteNoFormatting shouldn't set text/html to clipboard event on paste. r=enndeakin
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbd4c0e017c8
Part 2. Add test. r=enndeakin
https://hg.mozilla.org/mozilla-central/rev/ac3511a43396
https://hg.mozilla.org/mozilla-central/rev/bbd4c0e017c8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Since this fix has automated coverage, I don't think manual testing would be of much help here.

Makoto, if you think Manual QA should instead be looking at this, feel free to flip the qe-verify flag or ni? me directly.
Flags: qe-verify-
(In reply to Andrei Vaida [:avaida], Desktop Release QA – please ni? me from comment #12)
> Since this fix has automated coverage, I don't think manual testing would be
> of much help here.
> 
> Makoto, if you think Manual QA should instead be looking at this, feel free
> to flip the qe-verify flag or ni? me directly.

It is unnecessary since I add mochitest.
You need to log in before you can comment on or make changes to this bug.