Last Comment Bug 764862 - Select all | Copy, on the about: page (xhtml), does not paste correctly
: Select all | Copy, on the about: page (xhtml), does not paste correctly
Status: VERIFIED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: seamonkey2.13
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-14 09:13 PDT by therube
Modified: 2012-07-06 06:02 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
verified


Attachments
Add crlf+" " so the paste is correct. (v1) (1.11 KB, patch)
2012-06-14 21:10 PDT, Edmund Wong (:ewong)
neil: review-
neil: ui‑review+
Details | Diff | Review
Add crlf+" " so the paste is correct. (v2) (2.08 KB, patch)
2012-06-15 19:51 PDT, Edmund Wong (:ewong)
neil: review+
Details | Diff | Review
Add crlf+" " so the paste is correct. (v3) (2.12 KB, patch)
2012-06-16 18:54 PDT, Edmund Wong (:ewong)
ewong: review+
bugspam.Callek: approval‑comm‑aurora+
Details | Diff | Review
Select all | Copy, on the about: page (xhtml), does not paste correctly (for comm-beta) (2.07 KB, patch)
2012-07-05 00:27 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Review

Description therube 2012-06-14 09:13:27 PDT
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20120613 Firefox/15.0a2 SeaMonkey/2.12a2
Build ID: 20120613013002

Steps to reproduce:

 
Open about:
Ctrl+A to select all
Ctrl+C to copy
 


Actual results:

 
When pasted, the layout is not formatted as expected
 


Expected results:

 
Pasted output should be laid out correctly

It's basically OK, except that the fields /User agent:/ & /Build identifier:/ run together, could stand a \n
 
 
This came about from Bug 752797 - Build ID absent from about: page
And relates in general to Bug 319141 - Copy from XHTML content always copies only plain text instead of formatted content (HTML or RTF)
Comment 1 Tony Mechelynck [:tonymec] 2012-06-14 15:24:59 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/16.0 Firefox/16.0a1 SeaMonkey/2.13a1 ID:20120614022149

I can reproduce the problem.

See http://mxr.mozilla.org/comm-central/source/suite/common/about.xhtml

Unlike the preceding lines, which are present in the form <li>...</li> in the XHTML source, the "User Agent" and "Build ID" lines are added by Javascript.

ewong: Would it make a difference if </script> then <script type=application/javascript> were added (on two different lines) between the block "if (ua) {...}" and the line "var buildID = ..." i.e. near current line 74?
Comment 2 Edmund Wong (:ewong) 2012-06-14 21:10:44 PDT
Created attachment 633388 [details] [diff] [review]
Add crlf+"    " so the paste is correct. (v1)

This patch is actually a workaround while bug #319141 is still open.
Comment 3 Philip Chee 2012-06-15 03:04:51 PDT
I think you should add a comment in the patch regarding bug 319141
> +        listItem.appendChild(document.createTextNode("\n    "));
Comment 4 neil@parkwaycc.co.uk 2012-06-15 05:23:53 PDT
Comment on attachment 633388 [details] [diff] [review]
Add crlf+"    " so the paste is correct. (v1)

>       // append user agent
>       var ua = navigator.userAgent;
>       if (ua) {
>         var list = document.getElementById("aboutPageList");
>         var listItem = list.appendChild(document.createElement("li"));
>         listItem.appendChild(document.createTextNode("&about.userAgent;"));
>         listItem.appendChild(document.createTextNode(ua));
>+        listItem.appendChild(document.createTextNode("\n    "));
Please can you append this text node to the list, rather than the item.
Can you also do it for the build ID for consistency?
Don't forget Ratty's comment of course.

>         var list = document.getElementById("aboutPageList");
>         var listItem = list.appendChild(document.createElement("li"));
Oops, duplicate variable declarations. We'll have to use let instead.
Comment 5 Edmund Wong (:ewong) 2012-06-15 19:51:10 PDT
Created attachment 633760 [details] [diff] [review]
Add crlf+" " so the paste is correct. (v2)
Comment 6 neil@parkwaycc.co.uk 2012-06-16 02:29:01 PDT
Comment on attachment 633760 [details] [diff] [review]
Add crlf+" " so the paste is correct. (v2)

[The blank lines were slightly different in the two places; they should look the same.]

>+      // append "\n    " to the aboutPageList children as a workaround
>+      // for bug 319141.
>+      list.appendChild(document.createTextNode("\n    "));
Please put this inside the if block [both times]. r=me with that fixed.
Comment 7 Edmund Wong (:ewong) 2012-06-16 18:54:38 PDT
Created attachment 633861 [details] [diff] [review]
Add crlf+" " so the paste is correct. (v3)
Comment 8 Edmund Wong (:ewong) 2012-06-17 06:22:52 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/1347d4a8e05e
Comment 9 Tony Mechelynck [:tonymec] 2012-06-17 12:14:31 PDT
    User agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/16.0 Firefox/16.0a1 SeaMonkey/2.13a1
    Build identifier: 20120617112337

In the clipboard copy, "User agent" and Build identifier" now appear on separate lines (as seen above), immediately after the other list items, the last one of which links to about:buildconfig.

=> VERIFIED for 2.13a1 Trunk.

2.12a2 Aurora is still affected.
Comment 10 Edmund Wong (:ewong) 2012-06-17 19:15:34 PDT
Comment on attachment 633861 [details] [diff] [review]
Add crlf+" " so the paste is correct. (v3)

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:
Comment 11 Edmund Wong (:ewong) 2012-06-17 19:16:41 PDT
[Approval Request Comment]
Regression caused by (bug #): 764862
User impact if declined: Minor/trivial
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch: None

(Once again, forgot to fill these in.)
Comment 12 Justin Wood (:Callek) 2012-07-04 22:09:30 PDT
...this failed to apply for beta, care to adjust patch and land yourself?
Comment 13 Edmund Wong (:ewong) 2012-07-05 00:27:41 PDT
Created attachment 639245 [details] [diff] [review]
Select all | Copy, on the about: page (xhtml), does not paste correctly (for comm-beta)
Comment 14 neil@parkwaycc.co.uk 2012-07-05 00:32:52 PDT
(In reply to Tony Mechelynck from comment #9)
> 2.12a2 Aurora is still affected.
So presumably 2.11 doesn't have bug 752797 so there's nothing to do there?
Comment 15 Edmund Wong (:ewong) 2012-07-05 00:55:49 PDT
Comment on attachment 639245 [details] [diff] [review]
Select all | Copy, on the about: page (xhtml), does not paste correctly (for comm-beta)

Did not need to patch comm-beta, so there's no need to push to comm-beta.
Removing review and other flags to this patch.
Comment 16 Edmund Wong (:ewong) 2012-07-05 01:18:37 PDT
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/c23fbddb47f4

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