Closed Bug 1639972 Opened 4 years ago Closed 4 years ago

Table is not kept when pasting to Google docs

Categories

(Core :: DOM: Selection, defect, P1)

Product:
Core
Component:
DOM: Selection
Platform:
Desktop
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
83 Branch
Project Flags:
Webcompat Priority ?
Tracking Flags:
Tracking Status
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- wontfix
firefox82 --- wontfix
firefox83 --- fixed

People

(Reporter: clara.guerrero, Assigned: mbrodesser-Igalia)

References

(Blocks 2 open bugs,
URL
)

Details

Attachments

(4 files, 5 obsolete files)

ff vs chrome table.mov
1.91 MB, video/quicktime
Details
minimal-table.html
200 bytes, text/html
Details
minimal-contentEditable-div.html
36 bytes, text/html
Details
Bug 1639972: don't insert a node in `InsertNodeIntoProperAncestorWithTransaction` when there's no proper ancestor. r=masayuki
47 bytes, text/x-phabricator-request
Details | Review
Attached video ff vs chrome table.mov —

[Affected versions]
Release 76.0 (64-bit)
Beta 77.0b8 (64-bit)
Firefox Nightly 78.0a1 (2020-05-21) (64-bit)

[Affected platforms]
( macOS 10.14 (Mojave))
(Ubuntu 18.04 x64)

[Steps to reproduce]

1- Launch the Firefox Browser and visit https://en.wikipedia.org/wiki/Game_of_Thrones or https://www.w3schools.com/html/html_tables.asp
2- Open https://docs.google.com/document
3- Select two rows from table from step 1 and copy
4- Paste it in the word tab from step 2
5- Data is pasted as a text paragraph.
6- Launch Chrome and open link from step 2
7- Paste your selection
8- Table format is kept.

Also, if pasting table from one window to a different window,it is also pasted correctly (for example from nightly to release/beta), the problem is when pasting table from another tab of the same window.

[Expected result]
FF should keep the table's format when pasting in an online editor.

[Actual result]
Format is lost when pasting.

Severity: -- → S3
Blocks: 1636028, 1636983
No longer depends on: 1636028, 1636983
Summary: Table format is not kept when pasting to an online word editor. → Table format is not kept when pasting to Google docs
Webcompat Priority: --- → ?

Also, if pasting table from one window to a different window,it is also pasted correctly (for example from nightly to release/beta)

On Ubuntu 18.04 this also doesn't work when copy-pasting from Nightly to Developer Edition.

Severity: S3 → --

Interesting, this works when selecting the whole table pressing Ctrl+Click for each table cell, but doesn't when drag-selecting all cells.

Edit: it works with the minimal example, but not with Google Docs.

After click-selecting:
"text/html" flavor:
<meta http-equiv="content-type" content="text/html; charset=utf-8"><table><tbody><tr><td>x</td><td>y</td></tr><tr><td>u</td><td>v</td></tr></tbody></table>

"text/_moz_htmlcontext" flavor:
<html><body><table><tbody></tbody></table></body></html>

"text/_moz_htmlinfo" flavor:
0,0

After drag-selecting:
"text/html" flavor:

<meta http-equiv="content-type" content="text/html; charset=utf-8"><table><tbody><tr><td>x</td>
        <td>y</td>
    </tr>
    <tr>
        <td>u</td>
        <td>v</td></tr></tbody></table>

"text/_moz_htmlcontext" flavor:
<html><body><table><tbody></tbody></table></body></html>

"text/_moz_htmlinfo" flavor:
2,2

So only the "text/_moz_htmlinfo" flavor differs (apart from whitespace).

Given the flavors of the new clipboard API spec, we could consider dropping "text/_moz_htmlcontext" and "text/_moz_htmlinfo". Henri, any thoughts about this?

Flags: needinfo?(hsivonen)

I don't know what text/_moz_htmlinfo is supposed to do. Does it convey information that on Windows is part of the header in the CF_HTML flavor?

Flags: needinfo?(hsivonen)

In my understanding, "text/_moz_htmlcontext" is used internally to decide whether pasted editor needs to create which type of parent element or not. E.g., copying only list-items. So, dropping it may change the traditional behavior. (But I don't know whether we should expose it for web apps or not.)

"text/_moz_htmlinfo" is handled here to create a document fragment.
https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/editor/libeditor/HTMLEditorDataTransfer.cpp#2999-3029
In the clicking case, this does nothing.

Some analysis: https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/dom/base/nsDocumentEncoder.cpp#907 is reached only when copying after drag-selecting, not when click-selecting.

About c7 and c8: not sure yet. Need to understand more of the relevant code. But of course, if "text/_moz_htmlcontext" or other flavors were removed, the editor-code would need to be adapted in order to not break existing functionality.

Assignee: nobody → mbrodesser
Priority: -- → P1

Copy&pasting a table within Chrome is already broken, if the selected elements don't contain the <table> element. That is:

  1. In the failing case, the clipboard contains:
<meta http-equiv="content-type" content="text/html; charset=utf-8"><tr style="color: rgb(0, 0, 0); font-family: &quot;Times New Roman&quot;; font-size: medium; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><td>x</td><td>y</td></tr><tr style="color: rgb(0, 0, 0); font-family: &quot;Times New Roman&quot;; font-size: medium; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><td>u</td><td>v</td></tr>
  1. In the successful case, it contains:
<meta http-equiv="content-type" content="text/html; charset=utf-8"><br class="Apple-interchange-newline"><table style="font-family: &quot;Times New Roman&quot;; letter-spacing: normal; orphans: 2; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><tbody><tr><td>x</td><td>y</td></tr><tr><td>u</td><td>v</td></tr></tbody></table>

The most reliable way to reproduce 1) was drag-selecting all table cells from the right bottom up to the left top not going beyond the left side of character "x" of the minimal table. Sometimes this was also possible by selecting from the top left cell to the bottom right one, but I couldn't determine reliable steps.
2) Worked usually when drag-selecting from "x" to "y".

(In reply to Henri Sivonen (:hsivonen) from comment #7)

I don't know what text/_moz_htmlinfo is supposed to do. Does it convey information that on Windows is part of the header in the CF_HTML flavor?

AFAICT, it doesn't.

OS: Unspecified → All
Hardware: Unspecified → Desktop

Findings:

1) On try, no test fails when the string corresponding to "text/_moz_htmlinfo" is changed to always correspond to "0,0". That is, it has no test-coverage.

2) The correct fix could be to adapt nsDocumentEncoder::SerializeSelection to serialize less content for
the "text/_moz_htmlcontext" flavor. That is, instead of

<html><body><table><tbody></tbody></table></body></html>

only

<html><body></body></html>

This would preserve what other applications read from the clipboard (assuming they don't read any of the "text/_moz_" flavors). It would change what Gecko's editor parses in HTMLEditor::CreateDOMFragmentFromPaste. This would fix the click-selecting case, not the drag-selecting case, because the latter creates "text/_moz_htmlinfo" with value "2,2".

3) The "text/_moz_htmlinfo" flavor seems to only make sense when its numbers are both zero or differ (this invariant deserves an assertion).
The latter, happens for instance when

data:text/html,o|uter<div>inn|er</div>

is selected and copied to the clipboard.

(In reply to Mirko Brodesser (:mbrodesser) from comment #12)

1) On try, no test fails when the string corresponding to "text/_moz_htmlinfo" is change to always correspond to "0,0". That is, it has no test-coverage.

Well, neither comm-central nor BlueGriffon refers it. So, if no other applications refer it, we could change it. But I have no idea how other applications like Office handle HTML data copied by Gecko. So, you should try to check with some other native applications too.

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #13)

(In reply to Mirko Brodesser (:mbrodesser) from comment #12)

1) On try, no test fails when the string corresponding to "text/_moz_htmlinfo" is change to always correspond to "0,0". That is, it has no test-coverage.

Well, neither comm-central nor BlueGriffon refers it. So, if no other applications refer it, we could change it. But I have no idea how other applications like Office handle HTML data copied by Gecko. So, you should try to check with some other native applications too.

I don't intend to change it (the comment was edited some time after its creation, that might've caused confusion, sorry) anymore. That statement is merely stating that it's not test-covered.

Other native applications seem to rely on the "text/html" flavor. AFAIK, other browsers (e.g. Chrome) export the relevant data entirely to that flavor. In order to fix this ticket, I'll attempt to fix the exports for the existing flavors. If we decide to drop the "text/_moz_*" flavors, that should be done separately.

Attachment #9160035 - Attachment is obsolete: true

The invariant mentioned in #c12 ("The "text/_moz_htmlinfo" flavor seems to only make sense when its numbers are both zero or differ (this invariant[...]) is wrong. The following example with selection in two siblings shows this:

data:text/html,<body><div>X|select</div><div>this|Y</div></body>
Attachment #9160034 - Attachment is obsolete: true
Attachment #9160033 - Attachment is obsolete: true
Attachment #9160032 - Attachment is obsolete: true
Attachment #9160031 - Attachment is obsolete: true

This issue is reproducible on Nightly 80.0a1 (2020-07-22)(32-bit). I'll change flags accordingly.

status-firefox80: --- → affected

Reproduced on the latest Firefox 81 beta and Nightly 82.0a1 on Windows 7 x64.

status-firefox81: --- → affected
status-firefox82: --- → affected

Interestingly, the problem does not occur with the minimal example, when the target document has document.designMode = "on".

The runs with document.designMode = "on" and = "off" differ at HTMLWithContextInserter::InsertContents:

for "off", insertedPoint.IsSet() is true,
for "on", it's false.

Therefore, presumably the bug is around InsertNodeIntoProperAncestorWithTransaction.

Corresponding Pernosco runs:

The previous behavior was added in bug 858918. However, it was wrong
when trying to insert a <tr> into a <div contenteditable>.

After this change, the test in bug 858918 still passes, so that specific
functionality doesn't break. However, the code is complex and one can't
exclude that other functionality breaks. The previous fix already
mentioned the same concern
https://bugzilla.mozilla.org/show_bug.cgi?id=858918#c25.

With this change,
https://searchfox.org/mozilla-central/rev/35245411b9e8a911fe3f5adb0632c3394f8b4ccb/editor/libeditor/HTMLEditorDataTransfer.cpp#938
doesn't insert the <tr>, but climbs up the <tr>'s ancestors, so that
a <table> is inserted. When designMode = "on", this was already
previously the case.

The new code seems more correct, including pasting tables to
contenteditable elements working now. Therefore, it seems worth
submitting this patch, despite being unable to exclude regressions by
pure reasoning.

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ad8640ee3af
don't insert a node in `InsertNodeIntoProperAncestorWithTransaction` when there's no proper ancestor. r=masayuki

https://hg.mozilla.org/mozilla-central/rev/3ad8640ee3af

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox83: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

Hi,
I just tested this on Firefox 84.0b6 and this still happens.

STR:
I created a table on an excel sheet, with filled colour and borders.
I copied the columns or just the cells.
I pasted it in a google sheet.

Actual result:
The format is not kept, Firefox just copied the content.

Expected result:
Format and content should be pasted.

Flags: needinfo?(mbrodesser)

Florencia: this issue is about copy-pasting a table from one browser-tab to Google Docs, and contenteditable content in general. It fixes keeping the table, but doesn't change or fix anything for the table's colors and borders. That is, with this fix, the pasted table in Google Docs should contain multiple clickable cells, but not necessarily borders or colors. The ticket's title was misleading, I'll rephrase it.

Flags: needinfo?(mbrodesser)
Summary: Table format is not kept when pasting to Google docs → Table is not kept when pasting to Google docs

I've tested it on 84.0b6 and it meets the expectations of #c29.

Thanks Mirko for the clarification, sorry for the extra trouble.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: