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)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: arni2033, Assigned: m_kato)
References
Details
Attachments
(2 files)
9.04 KB,
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
3.32 KB,
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
>>> 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>
Assignee | ||
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24734fbaf04c2adcd63187bff31eefd6b62ce9d7
Assignee | ||
Comment 3•7 years ago
|
||
Assignee: nobody → m_kato
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
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)
Assignee | ||
Updated•7 years ago
|
Attachment #8832780 -
Flags: review?(enndeakin)
Comment 6•7 years ago
|
||
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 7•7 years ago
|
||
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
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac3511a43396 https://hg.mozilla.org/mozilla-central/rev/bbd4c0e017c8
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 12•7 years ago
|
||
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-
Assignee | ||
Comment 13•7 years ago
|
||
(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.
Description
•