Closed Bug 1433073 Opened 6 years ago Closed 4 years ago

Copy-pasting multiple click-selected table rows to a WYSWYG editor loses linebreaks

Categories

(Core :: DOM: Serializers, defect, P2)

Product:
Core
Component:
DOM: Serializers
Version:
58 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox-esr68 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- fixed

People

(Reporter: matteo.sindona, Assigned: mbrodesser-Igalia)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(7 files, 1 obsolete file)

Immagine.png
66.68 KB, image/png
Details
CORRECT.png
104.81 KB, image/png
Details
WRONG.png
34.69 KB, image/png
Details
minimal-example.html
131 bytes, text/html
Details
Bug 1433073: part 1) Update documentation of EventUtils' `synthesizeMouse` and `synthesizeKey`. r=hsivonen
47 bytes, text/x-phabricator-request
Details | Review
Bug 1433073: part 2) Add test. r=hsivonen
47 bytes, text/x-phabricator-request
Details | Review
Bug 1433073: part 3) Mimic old behavior of computing `mCommonInclusiveAncestors`. r=hsivonen
47 bytes, text/x-phabricator-request
Details | Review
Attached image Immagine.png — 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180118215408

Steps to reproduce:

There is a huge problem with Firefox 58.0 version.

When I copy a table from a website into another, the table is copied in the wrong way, because everything is in one single row. All the columns are merged together and they are not taken into consideration.

The software used in the website I have the problem is Discuz!, but I guess the problem also appears with other softwares. 


Actual results:

All the columns are merged together and they are not taken into consideration. (see attachment)


Expected results:

Attached is the comparison of right (up to Firefox 57.0.4) and wrong (Firefox 58.0)
Hi Matteo and thanks for reporting this.

Since I don't have the tool you mentioned in comment 0, it would be very helpful if you could help us with some information.

Could you please retest this with the latest Nightly version and report back the results? When doing this, please use a new clean Firefox profile (https://goo.gl/AWo6h8), maybe even safe mode (https://goo.gl/AR5o9d), to eliminate custom settings as a possible cause.

Also, it would be very helpful if you could use mozregression (https://mozilla.github.io/mozregression/) in order to identify the specific commits which regressed this. Please let me know if you have any question regarding this.

Thanks!
Component: Untriaged → Editor
Product: Firefox → Core
(In reply to Carmen Fat [:carmenf] from comment #1)
> Hi Matteo and thanks for reporting this.
> 
> Since I don't have the tool you mentioned in comment 0, it would be very
> helpful if you could help us with some information.
> 
> Could you please retest this with the latest Nightly version and report back
> the results? When doing this, please use a new clean Firefox profile
> (https://goo.gl/AWo6h8), maybe even safe mode (https://goo.gl/AR5o9d), to
> eliminate custom settings as a possible cause.
> 
> Also, it would be very helpful if you could use mozregression
> (https://mozilla.github.io/mozregression/) in order to identify the specific
> commits which regressed this. Please let me know if you have any question
> regarding this.
> 
> Thanks!


Hello Carmen,

I tried it using the latest Nightly version and using the safe mode, and unfortunately I still get the problem.

I have no idea how to use mozregression, but I remember that the same problem happened some years ago (some versions between Firefox 10 and 20) with one or two Firefox versions and then disappeared with the newer version. If you think that would help to track and solve the problem, I can try to figure out which of the oldest versions had the same problem.

Also, if you want to test yourself, I created a test account in my website, you can login with it and try youself.

Go here and login: http://results.totallympics.com/forum.php?mod=post&action=newthread&fid=37

Username: test 
password: test123

Now open this link in another tab and copy the table (whole table or just some cells, it's not important): https://data.fis-ski.com/dynamic/results.html?sector=NK&raceid=2189

You will get the problem with Firefox 58.0. With all the previous versions of Firefox (at least from 20.0 on, as I said around 2012-2013 some versions had the same problem) the table was copied correctly in the box.

Let me know if I can help you with some more information.

Thank you!

Matteo
Edit: I have tried to install all the previous versions of Firefox and this problem appeared in Firefox versions until the version 14.0.1. From version 15.0 on (until version 57.0.4) the problem did not appear, and now is back in version 58.0 (and 58.0.1 is still the same).

Let me know if I can give you some additional information.

Thank you!

Matteo
Hi Matteo,

Thanks for the information and the test case you provided, it is appreciated.

I followed the steps you mentioned in comment 2 and logged into the test account. However, the table is copied and pasted correctly into the editable area.
I tried this with both Firefox Release v. 58.0.1 and an older release on which should have also been reproducible (Firefox v. 3.6), using two distinct machines with Windows 10 x64. I didn't reproduce the issue on the previously mentioned builds and machines.
	
Since I can't reproduce the issue, it would be great if you could use the mozregression in order to find out exactly what caused the regression.

Here are the steps to install and use the mozregression tool:
1. Install mozregression tools from here: http://mozilla.github.io/mozregression/install.html 
2. Use the terminal commands mentioned in the tutorial and after that, use the next command (without commas) to start testing:
"mozregression --good year-month-day --bad year-month-day".  So in your case, you have to run the following command: mozregression --good 2017-08-02 --bad 2018-01-16
3. When a Firefox window is opened, try to reproduce the issue. 
4. If the issue is reproducible, type "bad" in the terminal and hit "Enter", if is good type "good". 
5. Please provide us the pushlog link after no more builds are opened by the tool.

Please let me know if you have any question regarding this and thanks for your patience.
Flags: needinfo?(matteo.sindona)
Hello Carmen,

I am sorry but I have no idea what a "terminal commands" are, so I can not use Mozregression.

Are you sure you are not facing the problem with Firefox 58.0.1 ?

I keep getting it and all the users who are using my website are getting the same problem, so it seems very strange to me that the problem is not appearing on your device.


I have tried with version 3.6 and everything was ok, the issue was appearing between Firefox versions 8 up to 14. From version 15 on, everything was ok again.

The problem with version 58 is that the table is copied into the text box with the correct data, but everything is on one single row. 

So for example if I copy a table made of 3 rows and 3 columns, when I paste it in the text area I get a table with 9 columns and 1 row, instead of 3 rows and 3 columns.

I will try to upload two new files here to show you the difference between version 57 and version 58.

Please let me know if you see a different table when you copy the same data from the same website with version 58.

Thank you,

Matteo
Flags: needinfo?(matteo.sindona)
Attached image CORRECT.png — 
Correct (Firefox version 57)
Attached image WRONG.png — 
Wrong (Firefox 58.0.1)
Here are the two attachments.

I did exactly the same thing, and you can see the two different outcomes.

I copied the first 10 rows of the table you can find here https://data.fis-ski.com/dynamic/results.html?sector=NK&raceid=2189, selecting 10 rows and all the 8 columns, keeping CTRL pressed on my keyboard while selecting.

Then I paste it in the text box here http://results.totallympics.com/forum.php?mod=post&action=newthread&fid=37

In the correct version (Firefox 57), you can see that the table has 10 rows and 8 columnns.

in the wrong version (Firefox 58.0.1), you can see that the table has just 1 row and a lot of columns (probably 80, because it's 10x8). It's like the system doesn't recognize when a new row starts so everything is in the same row.

Can you please show me a screenshot of the table you get when you paste the content with Firefox 58 ?


Thank you!

Matteo
Hi again Matteo,

No wonder I wasn't able to reproduce the issue since I only tried copying through the old fashioned ways (by shortcut and context menu). 
I was able to reproduce the issue after pressing the "Ctrl" key while manually selecting the table. 
Sometimes a small detail like that can make huge difference :)

Since it was reproducible on my side, I performed a regression using mozregression tools.
Here are the results:
14:59.60 INFO: Last good revision: 63747de53588638215ecab23650625595ede4e61
14:59.60 INFO: First bad revision: b0a3a621f5c85b5e9a4eb97de77fcb2558ac17c6
14:59.60 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=63747de53588638215ecab23650625595ede4e61&tochange=b0a3a621f5c85b5e9a4eb97de77fcb2558ac17c6

Henri, as the author of the patch included in the push above, can you please take a look at this issue?
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
status-firefox58: --- → affected
status-firefox59: --- → affected
status-firefox60: --- → affected
Ever confirmed: true
Flags: needinfo?(hsivonen)
That's a great news !

I hope a Firefox developer can fix this problem.

It is very important for our website because all the contributors are working with Firefox, which is the best browser to deal with tables, and we use tables a lot around the website.

Unfortunately this problem is huge for us, so right now we have blocked the Firefox updates and we are all still using Firefox 57, but I really hope the problem can be fixed so we can start updating Firefox again very soon.

Let me know if I can help you with more information.

Thank you,

Matteo
Works for me on Linux, so I expect this is specifically about the "context" part in the Windows clipboard format.
(In reply to Henri Sivonen (:hsivonen) from comment #11)
> Works for me on Linux, so I expect this is specifically about the "context"
> part in the Windows clipboard format.

I was going by the earlier steps to reproduce.

The ctrl selection described in comment 8 is indeed broken on Linux, too, so this isn't about the Windows clipboard format.
Hello Henri,

sorry for not specifying about the CTRL selection before.

Is there any way this problem can be fixed in the next Firefox updates ?

Thank you,

Matteo
On Linux when copying the first 8 rows using ctrl selection, I see the plain-text clipboard flavor generate line breaks and I see this for the HTML flavor:
<table class="footable table-datas table-withpadding" cellspacing="0" cellpadding="0"><tbody><tr><td class="i0" align="right"> 1</td><td class="i0" align="right"> 7</td><td class="i0" align="right"> 100323</td><td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=171541&amp;type=result">GERARD Antoine</a> </td><td class="i0" align="center">1995 </td><td class="i0" align="center">FRA </td><td class="i0" align="right"> 12:00.2</td><td class="i0" align="right">  </td></tr><tr><td class="i1" align="right"> 2</td><td class="i1" align="right"> 1</td><td class="i1" align="right"> 100300</td><td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=170578&amp;type=result">SOETVIK Sindre Ure</a> </td><td class="i1" align="center">1992 </td><td class="i1" align="center">NOR </td><td class="i1" align="right"> 12:07.3</td><td class="i1" align="right"> +7.1</td></tr><tr><td class="i0" align="right"> 3</td><td class="i0" align="right"> 9</td><td class="i0" align="right"> 609</td><td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=69283&amp;type=result">LAHEURTE Maxime</a> </td><td class="i0" align="center">1985 </td><td class="i0" align="center">FRA </td><td class="i0" align="right"> 12:10.0</td><td class="i0" align="right"> +9.8</td></tr><tr><td class="i1" align="right"> 4</td><td class="i1" align="right"> 6</td><td class="i1" align="right"> 1706</td><td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=137790&amp;type=result">REHRL Franz-Josef</a> </td><td class="i1" align="center">1993 </td><td class="i1" align="center">AUT </td><td class="i1" align="right"> 12:10.7</td><td class="i1" align="right"> +10.5</td></tr><tr><td class="i0" align="right"> 5</td><td class="i0" align="right"> 3</td><td class="i0" align="right"> 100167</td><td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=161576&amp;type=result">ILVES Kristjan</a> </td><td class="i0" align="center">1996 </td><td class="i0" align="center">EST </td><td class="i0" align="right"> 12:16.4</td><td class="i0" align="right"> +16.2</td></tr><tr><td class="i1" align="right"> 6</td><td class="i1" align="right"> 2</td><td class="i1" align="right"> 100314</td><td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=171251&amp;type=result">RIIBER Harald Johnas</a> </td><td class="i1" align="center">1995 </td><td class="i1" align="center">NOR </td><td class="i1" align="right"> 12:19.9</td><td class="i1" align="right"> +19.7</td></tr><tr><td class="i0" align="right"> 7</td><td class="i0" align="right"> 4</td><td class="i0" align="right"> 100529</td><td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=185784&amp;type=result">BJOERNSTAD Espen</a> </td><td class="i0" align="center">1993 </td><td class="i0" align="center">NOR </td><td class="i0" align="right"> 12:27.4</td><td class="i0" align="right"> +27.2</td></tr><tr><td class="i1" align="right"> 8</td><td class="i1" align="right"> 11</td><td class="i1" align="right"> 606</td><td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=69280&amp;type=result">BRAUD Francois</a> </td><td class="i1" align="center">1986 </td><td class="i1" align="center">FRA </td><td class="i1" align="right"> 12:35.5</td><td class="i1" align="right"> +35.3</td></tr></tbody></table>

When using non-ctrl selection, I get:
<table class="footable table-datas table-withpadding" cellspacing="0" cellpadding="0"><tbody><tr><td class="i0" align="right"> 1</td>
<td class="i0" align="right"> 7</td>
<td class="i0" align="right"> 100323</td>
<td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=171541&amp;type=result">GERARD Antoine</a> </td>
<td class="i0" align="center">1995 </td>
<td class="i0" align="center">FRA </td>
<td class="i0" align="right"> 12:00.2</td>
<td class="i0" align="right">  </td>
        </tr>
        

        <tr>
<td class="i1" align="right"> 2</td>
<td class="i1" align="right"> 1</td>
<td class="i1" align="right"> 100300</td>
<td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=170578&amp;type=result">SOETVIK Sindre Ure</a> </td>
<td class="i1" align="center">1992 </td>
<td class="i1" align="center">NOR </td>
<td class="i1" align="right"> 12:07.3</td>
<td class="i1" align="right"> +7.1</td>
        </tr>
        

        <tr>
<td class="i0" align="right"> 3</td>
<td class="i0" align="right"> 9</td>
<td class="i0" align="right"> 609</td>
<td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=69283&amp;type=result">LAHEURTE Maxime</a> </td>
<td class="i0" align="center">1985 </td>
<td class="i0" align="center">FRA </td>
<td class="i0" align="right"> 12:10.0</td>
<td class="i0" align="right"> +9.8</td>
        </tr>
        

        <tr>
<td class="i1" align="right"> 4</td>
<td class="i1" align="right"> 6</td>
<td class="i1" align="right"> 1706</td>
<td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=137790&amp;type=result">REHRL Franz-Josef</a> </td>
<td class="i1" align="center">1993 </td>
<td class="i1" align="center">AUT </td>
<td class="i1" align="right"> 12:10.7</td>
<td class="i1" align="right"> +10.5</td>
        </tr>
        

        <tr>
<td class="i0" align="right"> 5</td>
<td class="i0" align="right"> 3</td>
<td class="i0" align="right"> 100167</td>
<td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=161576&amp;type=result">ILVES Kristjan</a> </td>
<td class="i0" align="center">1996 </td>
<td class="i0" align="center">EST </td>
<td class="i0" align="right"> 12:16.4</td>
<td class="i0" align="right"> +16.2</td>
        </tr>
        

        <tr>
<td class="i1" align="right"> 6</td>
<td class="i1" align="right"> 2</td>
<td class="i1" align="right"> 100314</td>
<td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=171251&amp;type=result">RIIBER Harald Johnas</a> </td>
<td class="i1" align="center">1995 </td>
<td class="i1" align="center">NOR </td>
<td class="i1" align="right"> 12:19.9</td>
<td class="i1" align="right"> +19.7</td>
        </tr>
        

        <tr>
<td class="i0" align="right"> 7</td>
<td class="i0" align="right"> 4</td>
<td class="i0" align="right"> 100529</td>
<td class="i0"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=185784&amp;type=result">BJOERNSTAD Espen</a> </td>
<td class="i0" align="center">1993 </td>
<td class="i0" align="center">NOR </td>
<td class="i0" align="right"> 12:27.4</td>
<td class="i0" align="right"> +27.2</td>
        </tr>
        

        <tr>
<td class="i1" align="right"> 8</td>
<td class="i1" align="right"> 11</td>
<td class="i1" align="right"> 606</td>
<td class="i1"><a href="https://data.fis-ski.com/dynamic/athlete-biography.html?sector=NK&amp;competitorid=69280&amp;type=result">BRAUD Francois</a> </td>
<td class="i1" align="center">1986 </td>
<td class="i1" align="center">FRA </td>
<td class="i1" align="right"> 12:35.5</td>
<td class="i1" align="right"> +35.3</td></tr></tbody></table>
These serializations differ only in whitespace in places where whitespace should be insignificant. Furthermore, ctrl-selecting, copying and pasting into LibreOffice works.

This suggests the bug is really in our *paste* handing.
Upon pasting (in the ctrl selection case), we end up with "tr" as the fragment parsing context.
So the problem is in context creation after all:

ctrl-select context:
<html class="js csspositionsticky" lang="en"><body class=" "><div class="dcm-w" id="container"><div id="main"><div class="row"><div class="large-12 columns"><div class="bloc-tab"><table class="footable table-datas table-withpadding" cellspacing="0" cellpadding="0"><tbody><tr></tr></tbody></table></div></div></div></div></div></body></html>

normal-select context:
<html class="js csspositionsticky" lang="en"><body class=" "><div class="dcm-w" id="container"><div id="main"><div class="row"><div class="large-12 columns"><div class="bloc-tab"><table class="footable table-datas table-withpadding" cellspacing="0" cellpadding="0"><tbody></tbody></table></div></div></div></div></div></body></html>

Both are wrong, but the other causes less trouble.

Ehsan, what value is there in exporting a context other than <html><body></body></html> (for compatibility with other apps that want to see a context)?

1) Except for <select>, the HTML fragment parsing algorithm never cares about the ancestors beyond the immediate context node, so serializing the deep context seems useless.

2) The HTML elements that exhibit non-default behavior as context nodes are relatively few and the most clipboard-relevant ones are table-related. Yet, when selecting a part of the table, we always export markup representing a full table (with the few cells that were selected) to the clipboard, so as far as context generation goes, AFAICT, we never actually need to export more context than <html><body></body></html>.

3) For incoming HTML, it would probably be more robust to always ignore the incoming context and to run the HTML parser with "template" as the context element. This way the tree builder state is set based on the first start tag token.
Flags: needinfo?(hsivonen) → needinfo?(ehsan)
(In reply to matteo.sindona from comment #13)
> Is there any way this problem can be fixed in the next Firefox updates ?

We aren't in a position to promise any specific schedule at this point. This bug needs fixing soon, though.
(In reply to Henri Sivonen (:hsivonen) from comment #18)
> (In reply to matteo.sindona from comment #13)
> > Is there any way this problem can be fixed in the next Firefox updates ?
> 
> We aren't in a position to promise any specific schedule at this point. This
> bug needs fixing soon, though.


Hello Henri,

thank you very much for your time. I really hope the problem can be fixed as soon as possible.

I also have some other minor things I would like to be changed in the tables features using Firefox, can I also report them to you?

Thank you,

Matteo
For those who helped me with this and who got familiar with those websites, I have posted another issue for which I have used the same example. You can find it here: https://bugzilla.mozilla.org/show_bug.cgi?id=1438756

Thank you in advance for the time you will spend on it and I really hope you can fix those issues soon.
(In reply to Henri Sivonen (:hsivonen) from comment #17)
> So the problem is in context creation after all:
> 
> ctrl-select context:
> <html class="js csspositionsticky" lang="en"><body class=" "><div
> class="dcm-w" id="container"><div id="main"><div class="row"><div
> class="large-12 columns"><div class="bloc-tab"><table class="footable
> table-datas table-withpadding" cellspacing="0"
> cellpadding="0"><tbody><tr></tr></tbody></table></div></div></div></div></
> div></body></html>
> 
> normal-select context:
> <html class="js csspositionsticky" lang="en"><body class=" "><div
> class="dcm-w" id="container"><div id="main"><div class="row"><div
> class="large-12 columns"><div class="bloc-tab"><table class="footable
> table-datas table-withpadding" cellspacing="0"
> cellpadding="0"><tbody></tbody></table></div></div></div></div></div></
> body></html>

OK, so we are getting a <tr> in the ctrl-click case but not in the normal selection case.  But what changed after the regression compared to before?

> Both are wrong, but the other causes less trouble.

Wrong how?  Can you explain please?

> Ehsan, what value is there in exporting a context other than
> <html><body></body></html> (for compatibility with other apps that want to
> see a context)?

I'm not quite sure if I understand your question.  The obvious answer is of course compatibility with the said apps that you mention but I think you're alluding to a different point that I'm not following?  I'd be cautious changing what context we export to the clipboard, changing how we consume the context ourselves is less questionable.

(But please note that I still don't understand where the regression comes from, so my first preference in the abstract is finding the root cause of the regression and fixing that, so that we go back to exporting the exact same context we used to before bug 1409951 landed.  Since I can't see that bug, and you haven't explained the regression itself, I don't feel like I can say much here with any confidence yet...)

> 1) Except for <select>, the HTML fragment parsing algorithm never cares
> about the ancestors beyond the immediate context node, so serializing the
> deep context seems useless.
> 
> 2) The HTML elements that exhibit non-default behavior as context nodes are
> relatively few and the most clipboard-relevant ones are table-related. Yet,
> when selecting a part of the table, we always export markup representing a
> full table (with the few cells that were selected) to the clipboard, so as
> far as context generation goes, AFAICT, we never actually need to export
> more context than <html><body></body></html>.
> 
> 3) For incoming HTML, it would probably be more robust to always ignore the
> incoming context and to run the HTML parser with "template" as the context
> element. This way the tree builder state is set based on the first start tag
> token.

I _think_ that makes sense based on my limited understand of how the parser works, but I think you know that side a lot better than I do.  Still this doesn't require us changing what context we put on the clipboard...
Flags: needinfo?(ehsan)
Once the problem will be identified and fixed, which will be the first version of Firefox which will not have this issue ? Can the problem be fixed immediately for the next release or it will be first fixed in the beta releases ?

Thank you,

Matteo
(In reply to :Ehsan Akhgari from comment #21)
> (In reply to Henri Sivonen (:hsivonen) from comment #17)
> > So the problem is in context creation after all:
> > 
> > ctrl-select context:
> > <html class="js csspositionsticky" lang="en"><body class=" "><div
> > class="dcm-w" id="container"><div id="main"><div class="row"><div
> > class="large-12 columns"><div class="bloc-tab"><table class="footable
> > table-datas table-withpadding" cellspacing="0"
> > cellpadding="0"><tbody><tr></tr></tbody></table></div></div></div></div></
> > div></body></html>
> > 
> > normal-select context:
> > <html class="js csspositionsticky" lang="en"><body class=" "><div
> > class="dcm-w" id="container"><div id="main"><div class="row"><div
> > class="large-12 columns"><div class="bloc-tab"><table class="footable
> > table-datas table-withpadding" cellspacing="0"
> > cellpadding="0"><tbody></tbody></table></div></div></div></div></div></
> > body></html>
> 
> OK, so we are getting a <tr> in the ctrl-click case but not in the normal
> selection case.  But what changed after the regression compared to before?

Before the regression, the context for the ctrl-select case was exactly the same as the above-quoted normal-select context.

> > Both are wrong, but the other causes less trouble.
> 
> Wrong how?  Can you explain please?

The content string contains the tags <table class="footable table-datas table-withpadding" cellspacing="0" cellpadding="0"><tbody> and the context contains them, too. They overlap and due to tables being special, the overlapping tags get ignored when parsing the content string.

Is generating stuff to be ignored by the consumer by design to accommodate different consumers?

> > Ehsan, what value is there in exporting a context other than
> > <html><body></body></html> (for compatibility with other apps that want to
> > see a context)?
> 
> I'm not quite sure if I understand your question.  The obvious answer is of
> course compatibility with the said apps that you mention but I think you're
> alluding to a different point that I'm not following?  I'd be cautious
> changing what context we export to the clipboard, changing how we consume
> the context ourselves is less questionable.

First, let's ignore everything except the table internals, select internals, xmp and plaintext. The parsing of the rest of HTML that can be copied from the content area shouldn't (per spec) change depending on what the context node is. Therefore, it should be sufficient to give <body> as the context and <html> in case other apps want to see the ancestor of <body> explicitly.

Second, when we export a partial table range, we export the content string in a form that fills in the tags to make it a full table. Is the expectation that the consumers parse out a full table, or is it some weird compat feature that content string contains markup for a full table, but if the consumer takes into account the context, the outer parts of the content string get ignored? (I.e. if context is <html><body><table><tbody></tbody></table></body></html> the <table> and <tbody> tags included in the context string get ignored.)

If exporting a full table but causing the outer bits to be ignored is a feature, can we special-case context generation for the small number of table-related elements instead of walking up the actual DOM for context generation? Generating the context from the actual DOM seems to be a continuous source of bugs.

That is, can we replace the current context generation logic with the following?

If the nearest common ancestor of the range end points is:
<xmp> or <plaintext>, don't export the HTML flavor to the clipboard--only plain text
<tr>, generate the context <html><body><table><tbody><tr></tr></tbody></table></body></html>
<tbody>, <thead> or <tfoot>, generate the context <html><body><table><tbody></tbody></table></body></html>
<table>, generate the context <html><body><table></table></body></html>
<table>, generate the context <html><body><table></table></body></html>
<select>, generate the context <html><body><select></select></body></html>
otherwise, generate the context <html><body></body></html>

(Can we even select stuff inside a <select>?)

> (But please note that I still don't understand where the regression comes
> from

The change in bug 1409951 was not supposed to change the generation of context start--only the generation of context end. Yet, as we see here, the context start acquired an extra <tr> tag in the ctrl-select case. I don't understand why.

> bug 1409951 landed.  Since I can't see that
> bug

CC added.
Flags: needinfo?(ehsan)
(In reply to matteo.sindona from comment #22)
> Once the problem will be identified and fixed, which will be the first
> version of Firefox which will not have this issue ? Can the problem be fixed
> immediately for the next release or it will be first fixed in the beta
> releases ?

Whether it will be uplifted to beta is a decision that can be made only after we see how invasive the fix ends up being.
(In reply to Henri Sivonen (:hsivonen) from comment #24)
> (In reply to matteo.sindona from comment #22)
> > Once the problem will be identified and fixed, which will be the first
> > version of Firefox which will not have this issue ? Can the problem be fixed
> > immediately for the next release or it will be first fixed in the beta
> > releases ?
> 
> Whether it will be uplifted to beta is a decision that can be made only
> after we see how invasive the fix ends up being.


Thank you Henri.
Please keep me updated because unfortunately I do not understand much about what you guys have posted in the previous messages.

I really hope this problem can be fixed as soon as possible.

Thanks!
Also, I don't know if this is useful for you, but doing some tests I have found out that the problem is caused from copying the table and not from pasting it.

In fact, if I copy the table with Firefox 58.0 and I paste it in Firefox 57.0 (or any previous version), the issue appears anyway, but the issue does not appear if I copy and paste the table from Firefox 57.0 (or any previous version). So the problem should be in copying from Firefox 58.0, pasting is not the problem.

I hope this helps.
Hello,

any news about this issue?

Thank you!
Regressed by: 1409951
Component: Editor → Serializers
Priority: -- → P2

Hello,

is there any way to have this fixed ?

Thank you!

The workaround is to Ctrl-V to a LO Calc (Gdocs, Excel) and then to copy from it as a formatted table.

Yeah, I am aware of that workaround. Another workaround is to select the tables with the mouse WITHOUT ctrl selection. But would it be possible to fix the bug in the future so we can still copy tables using ctrl like we were doing before Firefox 58.0 ?

Thanks!

I'm ready to donate 25 € to Mozilla when fixing this bug.

Can you post a publicly accessible table where this problems reproduces? The link (https://data.fis-ski.com/dynamic/results.html?sector=NK&raceid=2189) in comment 2 now seems to use divs.

Hello Tom, you are right, that website changed its table format in the while.

Please use this link to copy the table instead: https://dataride.uci.ch/Results/iframe/EventResults/220062?competitionId=59800&disciplineId=7

It is also important for me this problem to be fixed (I can't use Firefox with this bug) so I would be very grateful if you guys can fix it in the next release!

Thank you very much!

I've tried to reproduce this on Ubuntu 18.04 with Firefox 75.0 (64-bit) and didn't succeed. That is, the problem didn't occur.

I've used the link of c34 as input with CTRL and click selecting two consecutive rows.
Used Ctrl+c and Ctrl+v to paste the selected rows to https://froala.com/wysiwyg-editor/inline/ (because the login to the forum account provided above doesn't work).

Matteo, does the issue still occur for you? If so, does it also occur when pasting to https://froala.com/wysiwyg-editor/inline/? If not, can you please provide detailed steps to reproduce the issue nowadays?

Flags: needinfo?(matteo.sindona)

Hello Mirko,

thank you for your reply! Unfortunately, I still have the problem with Firefox 75.0 (64-bit), using Windows 10.

I also do not have the problem with froala link you posted, but I still have it on my website.

I have re-activated the test account so you can try yourself. Go here: https://results.totallympics.com/forum.php?mod=post&action=newthread&fid=71

And login with
Username "test"
Password "test123"

Copying and pasting using ctrl+c ctrl+v should give you the problem, unless something changes between Windows and Ubuntu.

But since on froala the problem does not occur, maybe I have to change some settings on my website?

Thank you and best regards,

Matteo

Flags: needinfo?(matteo.sindona)

Thanks for testing again and for re-activating the account.

With that account, the issue is reproducible on Ubuntu. Before pasting to the WYSIWYG editor, one needs to switch to advanced mode and uncheck the "Code" checkbox.

When selecting multiple consecutive rows (without pressing Ctrl), pasting works.

As Table-Click-Selection with Ctrl is a Firefox only feature, the issue requires more analysis to determine whether it's a bug in the forum of in Firefox. Both is possible.

Summary: Copying Tables in Firefox 58.0 problem → Copying click-selected tables in Firefox 58.0 problem
Summary: Copying click-selected tables in Firefox 58.0 problem → Copy-pasting multiple click-selected table rows to a WYSWYG editor loses linebreaks

Hello Mirko,

yeah, sorry I forgot to tell you to click on "Code" button before pasting the text, but you found it out by yourself.

I can edit any file of my website so if you have any suggestion on what should I do to have this problem fixed that would be amazing!

Thank you,

Matteo

Attached file minimal-example.html (obsolete) —
Attachment #9141694 - Attachment is obsolete: true

The proposal of c23 seems reasonable. However, in order to avoid regressions understanding why the fix mentioned there broke the behavior seems the safer way forward.

Assignee: nobody → mbrodesser

Found the cause of the regression: mCommonInclusiveAncestors is used after EncodeToString is called, but mCommonInclusiveAncestors isn't updated anymore as it was done previously.

Hello Mirko,

nice to hear that. Any chance this can be fixed in the future versions of Firefox ?

Thank you!

Check when copy-pasting multiple click-selected table rows, the
clipboard's "text/_moz_htmlcontext" flavor doesn't contain a <tr>.

Depends on D71980

This reverts a previously introduced regression. See the Bugzilla
comments of this bug.

The key consequence of this is change is that copy-pasting multiple
click-selected table rows now works again for some applications (and
shouldn't be broken for others). That's because the clipboard flavor
"text/_moz_htmlcontext" doesn't contain a superfluous "<tr>" anymore.

The fix could presumably be more elegant, but it would be hard to ensure
no other applications relying on the old behavior break.

Depends on D71981

(In reply to matteo.sindona from comment #43)

Hello Mirko,

nice to hear that. Any chance this can be fixed in the future versions of Firefox ?

Thank you!

I can't promise anything, but perhaps the fix can be merged to Nightly soon. This issue will track the status.

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a596f2de0950
part 1) Update documentation of EventUtils' `synthesizeMouse` and `synthesizeKey`. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/7e93722dbf4d
part 2) Add test. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/f32067e1ce26
part 3) Mimic old behavior of computing `mCommonInclusiveAncestors`. r=hsivonen
Flags: needinfo?(ehsan)
See Also: → 1640922
See Also: → 1640932
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: