Closed Bug 1380797 Opened 4 years ago Closed 4 years ago
"Copy text to clipboard" in Troubleshooting Info does not honor "Include account names" checkbox
In recent Nightly builds, (un)selecting the "Include account names" checkbox in Troubleshooting Information is not honored for the clipboard copy. STR: - Open About - Troubleshooting Information - Click Copy text to clipboard Expected results: General info is copied to clipboard, account names are not included unless checking the "Include account names" box. If "Include account names" is checked, additional account info is included in the clipboard copy, and a warning is displayed on top of the text. Actual results: Copied text to clipboard includes account names and no warning is displayed, either when checking or not checking the box (reports are identical). Last good build: Version: 54.0a1, Build ID: 20170225030215 First bad build: Version: 54.0a1, Build ID: 20170226030207 Possible regression of bug 1339811?
As you said, not only broken in the (old) Earlybird 54a1 but also in TB 56 Daily. I hope Aceman can fix this.
Version: unspecified → 54 Branch
Ok, let's see because I think we specifically have a test for this that "Include account names" toggles the private data and that it makes a difference when copying to clipboard.
This is real. It seems the problem is that the button on the page uses a function copyContentsToClipboard() to copy the data to the clipboard and it copies also hidden cells. The test passes because it seems to use normal copying to clipboard (as with Ctrl+C) and that is clever enough to not copy display:none elements. Yes, this may have happened with bug 1339811 because FF does not have the hidden cells so does not need to know to skip them and this simplified code was copied to TB.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
(In reply to :aceman from comment #3) > This is real. > It seems the problem is that the button on the page uses a function > copyContentsToClipboard() to copy the data to the clipboard and it copies > also hidden cells. > > The test passes because it seems to use normal copying to clipboard (as with > Ctrl+C) and that is clever enough to not copy display:none elements. This is wrong. The test just uses the function getClipboardTransferable() which is a copy of copyContentsToClipboard() but the older version that was used in TB and has the support for hidden private date. So I need to rewire this.
This patch renames copyToClipboard() in export.js to copyContentsToClipboard() so that the function shadows the one in aboutSupport.js so that our copy in export.js is called on click of the button. This function now calls getClipboardTransferable() so the test can continue using this internally. So now the click of the button also calls the old copying and serializing that hides the private cells. This is only a quick fix. I suggest creating a new bug for cleaning up the rest of the code, because now the aboutSupport.js copied from m-c contains a new Serializer (that is nicer) but is unused for now in TB. export.js contains the old serializer (createTextForElement()) which supports private cells, but on the other hand does not remove cells with 'no-copy' class. So our copying differs from the one in Firefox (on the same about:support page). Some of this code should be merged.
Attachment #8892705 - Flags: review?(richard.marti)
Comment on attachment 8892705 [details] [diff] [review] 1380797.patch Does what it should. Is it sure that the function from export.js is always used?
Attachment #8892705 - Flags: review?(richard.marti) → review+
Thanks, I'll get this landed before the next Daily.
Pushed by email@example.com: https://hg.mozilla.org/comm-central/rev/241cf259fcd7 call TB specific copy function in about:support to hide private data when copying to clipboard. r=Paenglab
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Thanks, Ton, for reporting. This will be fixed in TB 56 beta coming out soon.
Target Milestone: --- → Thunderbird 56.0
(In reply to Richard Marti (:Paenglab) from comment #6) > Is it sure that the function from export.js is always used? Our function is declared later (export.js is included later tna aboutSupport.js in aboutSupport.xhtml) so it should be called. See bug 371174 comment 24. Anyway, this is a temporary solution for TB 54-56. I'm going to file the follow-up now.
You need to log in before you can comment on or make changes to this bug.