Closed Bug 47309 Opened 25 years ago Closed 25 years ago

Cut collapses whitespace

Categories

(Core :: Layout: Form Controls, defect, P2)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: neil, Assigned: akkzilla)

References

Details

Attachments

(1 file)

Initial patch (gives rough idea, probably won't apply since line #'s are off)
3.66 KB, patch
Details | Diff | Splinter Review
Multiple spaces between words cut as a single space. Whitespace at the beginning of lines is not cut at all.
I'll start with you...
Assignee: rods → akkana
Confirmed on 080204 Win98.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 95 → All
I need some clarification on how to reproduce this. Where are you seeing this? Plaintext, html? Pasting to plaintext, or html? To our app, or somewhere else?
i cant repro this on win98 using build from today. i did the following: 1. opened new composer window 2. entered the following text {5 spaces}how{5 spaces}now{5 spaces}brown{5 spaces}cow 3. return and more text, saved file 4. selected the line with spaces, selected edit|cut 5. pasted after the other text typed in -- pasted with spaces intact
Beppe and Akkana: My way to repro was just to type in the "Additional Comments" box on this page a line of text with several spaces between each word. I cut it, then pasted it in the same box. It's still happening for me. Also, if for some reason it pastes the text twice, see bug 47322.
ahh -- it's happening within textareas and text fields; it is not happening in Composer or in Mail compose -- Akkana, should this go to mjudge or is this still yours?
This looks like the old "whether to include <body> in the selection or not when copying" dilemma. The only indication of plaintext is a style attribute in the body tag, so without that, we have no idea that whitespace is supposed to be honored. I guess we'll have to go back to including the body tag in the copied output. I forget why we stopped doing that (it fixed some other bug) -- I'll have to check cvs records and see, and try to reconcile the two bugs. Cc'ing Ben just in case he remembers something about the earlier bug, though I don't think he was involved in that change.
Status: NEW → ASSIGNED
Keywords: correctness, nsbeta3
Target Milestone: --- → M18
akk, no, I don't know anything about that.
setting to nsbeta3+ -- Akkana can you please add the priority to this one
Whiteboard: [nsbeta3+]
P3 (nontrivial, due to resolving issues with the other bug, but not huge) seems appropriate, so I'll leave it there.
thanks akk
Whiteboard: [nsbeta3+] → [nsbeta3+][p:3]
Argh. This is turning out to be much harder than expected -- it turns out that this is an ender lite issue. Copy used to get the style from the document's body node, which would tell it whether it was preformatted and thus needed to preserve whitespace. With ender lite, this information is in an anonymous div which isn't seen by the top-down iteration used in nsDocument::ToXIF. Joe has a bug to change the selection output methods to use a selection iterator, but that alone won't solve this bug either -- because I think the selection code gets the XIF from the document, rather than going through the selection or the document encoder. (Which will have to change anyway, soon, when we move away from the "iterate over the whole document" model to the "get output directly from the selection" model, and even more radically when/if we move away from the XIF output model.) Meanwhile, this will require architectural changes and is at severe risk for beta3.
Whiteboard: [nsbeta3+][p:3] → [nsbeta3+][p:3] AT RISK
Whiteboard: [nsbeta3+][p:3] AT RISK → [nsbeta3-][p:3] AT RISK
Target Milestone: M18 → Future
since this is architectual, and we are so close to nsbeta3 making this nsbeta3-, putting out to future and adding helpwanted
Updating QA contact.
QA Contact: ckritzer → bsharma
Keywords: rtm
Whiteboard: [nsbeta3-][p:3] AT RISK → [nsbeta3-]
When noxif lands, this will become a lot easier, and it's certainly irritating from a user's point of view. Suggest we fix it for rtm.
agreed, this should be fixed along with noxif
Priority: P3 → P2
Whiteboard: [nsbeta3-] → [nsbeta3-][rtm+][p:2]
Target Milestone: Future → M19
Akkana, please include the required information per the rtm checkin rules
Whiteboard: [nsbeta3-][rtm+][p:2] → [nsbeta3-][rtm+ NEED INFO][p:2]
This is semi-dogfood: I keep running into it in various incarnations. For example, go to any bugzilla report that includes a patch, select some or all of the patch and try to paste it into an editor. Now you have to go through and add newlines and whitespace because it got removed. It's a major regression from 4.x and a major correctness problem. I don't know how long it will take to fix this, because it's dependant on noxif landing and my noxif branch doesn't currently build. I'd guess a few days of work after the noxif branch either lands or syncs with the trunk or NS branch.
Keywords: 4xp
Whiteboard: [nsbeta3-][rtm+ NEED INFO][p:2] → [nsbeta3-][rtm+][p:2]
re-adding need info
Whiteboard: [nsbeta3-][rtm+][p:2] → [nsbeta3-][rtm+ NEED INFO][p:2]
PDT marking this rtm-. The noxif bug where extra whitespace gets added is worse than this. BTW, this also seems to work ok in the 10/2 branch build.
Whiteboard: [nsbeta3-][rtm+ NEED INFO][p:2] → [nsbeta3-][rtm-][p:2]
Target Milestone: M19 → Future
setting to future per pdt team comments, btw I did retry this on win98 using the comm build from today and it works fine, i went to a bug that had a patch attached, selected it, selected copy and pasted it into this textarea and it pasted fine.
Phil and beppe: I don't see the copy-from-patch-attachment problem in today's build either, but I still see the originally reported problem: in a textarea (e.g. the Additional Comments field here in this bug report), type a few letters followed by several spaces followed by a few more letters, copy the whole thing and paste it into plaintext somewhere else, and note that all the spaces get collapsed to one.
ok, maybe this isn't readily apparent in the description of the bug, but this works as expected in 4.x and in IE, this is the ONLY browser app that works in this manner. It's just downright goofy. If you are trying to column data for example, you can't create just one 'template' and copy paste it. For example, try this in the comments textarea: one i added 5 spaces now copy what you just typed and paste it, it doesn't work please reconsider
Whiteboard: [nsbeta3-][rtm-][p:2] → [nsbeta3-][rtm NEED INFO][p:2]
Target Milestone: Future → M19
Note: this is difficult with the current output model, because the document doesn't provide enough information for the output system to know we're copying plaintext. But in the noxif model (slated to land soon), this should be much easier. Marking dependency.
Depends on: 50742
Usefully, I can copy from an email and paste into a textbox without losing spaces.
*** Bug 26093 has been marked as a duplicate of this bug. ***
Joe helped out with tracking down where to put in the new code. The document copy encoder already has code to determine whether it's in a text control. So I needed to 1. sandwich the html generated by the copy encoder in a <pre></pre>, so that when it's stored in the clipboard and later translated to plaintext, we'll still know that it should be treated as preformatted. 2. pass a preformatted flag through to the nsHTMLContentSerializer, so that it knows not to add prettyprint formatting around html nodes (especially br tags) which might be interpreted as part of the preformatted content. So the third part of the patch is 3. change the prettyprint methods in the content serializer to not emit formatting if the preformatted flag is set. Will attach patch; looking for comments and reviews from Joe and/or Johhny. This isn't a blocker for the noxif landing; this can land independantly after the big landing. Line numbers in the patch will probably be a little off because they're applied on top of another patch I got from Joe.
Whiteboard: [nsbeta3-][rtm NEED INFO][p:2] → [nsbeta3-][rtm NEED INFO][p:2] HAVE FIX, TESTING/REVIEWING
Turns out this was fixed by the noxif landing on the branch. Checking now to see if it's also already fixed on the trunk ...
Whiteboard: [nsbeta3-][rtm NEED INFO][p:2] HAVE FIX, TESTING/REVIEWING → [nsbeta3-][rtm NEED INFO][p:2] FIXED ON BRANCH
fixed on branch, so removing rtm droppings.
Keywords: rtm
Whiteboard: [nsbeta3-][rtm NEED INFO][p:2] FIXED ON BRANCH → FIXED ON BRANCH
The fix is now in the trunk as well.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Whiteboard: FIXED ON BRANCH
Sorry for the spam! But apparently all these closed bugs need to have their target milestones changed since M19 and M20 are going away. Since they're allready closed, I'm choosing M18.
Target Milestone: M19 → M18
Verifying Windows98 2001-02-06-08-Mtest RedHat6.2 2001-02-06-08-Mtest MacOS 8.x 2001-02-06-10-Mtest
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: