Closed Bug 30763 Opened 25 years ago Closed 24 years ago

comment added after plaintext reply is inside reply

Categories

(Core :: DOM: Editor, defect, P3)

x86
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: akkzilla, Assigned: mozeditor)

References

Details

(Keywords: helpwanted, Whiteboard: [nsbeta3-][p:3] critical to mozilla 0.9)

Attachments

(1 file)

Start up the plaintext editor. Copy some plaintext in another app, and PasteAsQuotation in the editor. Now click somewhere in the middle of the quoted text, and sweep down to the end of the file. Hit delete to delete the last part of the quoted text. At this point, if you do an OutputHTML, the end of the file will look like this: &gt; Profile Manager : Command Line Options : End<br> <br> </pre> </body> </html> i.e. the new br got inserted inside the pre, not outside as should have happened. Now type an "a" and verify that it's inside the quoted block: a<br> </pre> </body> </html> This is bad, because deleting to the end of a quoted block and then adding text is a very common thing to do, and the result will be that the user will discover when he gets to the end of the line that his typed text does not wrap.
Another problem: if instead of typing 'a', you type a return, it splits the blockquote, even though there's nothing after the caret inside the blockquote, so you end up with: </pre> <br> asfd<br> <pre _moz_quote="true"> <br> </pre> </body> </html> This means that as you click into blank lines prior to typing, sometimes you will end up outside the pre, where you want to be, but sometimes you'll end up inside the pre, and then any long lines you type won't be wrapped. There's a slight font difference between these two places, but it's subtle and most people won't notice it or remember which font is which; they'll just wonder why it wraps sometimes and not other times.
M16 for now
Target Milestone: M16
Kathy -- I hope you don't mind if I change this to M15. It's a dogfood issue for plaintext mail, and if I'd discovered it a week earlier I would have lobbied to get it on the beta1 list. If Joe is that overloaded, maybe I can help with this (Joe and I have talked about it and we have some idea what's happening).
Target Milestone: M16 → M15
accepting bug.
Status: NEW → ASSIGNED
moving to M17
Target Milestone: M15 → M17
I'll try to help Joe look at this during M16. It might even be fixed already by one of his other changes to the edit rules. Can someone check and see if it's still happening?
Assignee: jfrancis → akkana
Status: ASSIGNED → NEW
Target Milestone: M17 → M16
It still happens, but things have changed a bit: now when I hit delete, no new br tag is added at all outside the pre tag, so there is no way to set the caret after the blockquote in order to add new text. This might be livable, since a user encountering that situation will probably hit return. Now, if I hit return, it still splits the pre and puts the br in the middle of the pre, which it should not. The pre is a <pre _moz_quote="true">. Perhaps the splitting code needs to be smart enough to detect that there's no more editable content after the current selection, and in that case, add a pre outside rather than inside. (Oof.) Joe, have you given this any thought? Should I try implementing that?
Status: NEW → ASSIGNED
probably the wrong bug to snarf from me. I'm working in this area anyway right now; i'll take a look.
Assignee: akkana → jfrancis
Status: ASSIGNED → NEW
fixed. Hitting return always gets you out of the mailquote now.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I'm still seeing this, while trying to split quotes or reply after a quote -- half the time my reply turns out to be inside the pre, meaning it isn't wrapped. It shouldn't be possible to type inside this pre. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
setting to m19
Target Milestone: M16 → M19
nominating for b3
Keywords: nsbeta3
Adding correctness and nsbeta3+
Keywords: correctness
Whiteboard: nsbeta3+
Whiteboard: nsbeta3+ → {nsbeta3+}
Whiteboard: {nsbeta3+} → [nsbeta3+]
setting priority in status whiteboard (medium)
Whiteboard: [nsbeta3+] → [nsbeta3+][p;3]
correcting typo in stat whiteboard, so that i can sort my bugs by the whiteboard
Whiteboard: [nsbeta3+][p;3] → [nsbeta3+][p:3]
m18
Target Milestone: M19 → M18
setting to future and adding helpwanted Need to triage bugs to meet the glidepath for pr3
Keywords: helpwanted
Whiteboard: [nsbeta3+][p:3] → [nsbeta3-][p:3]
Target Milestone: M18 → Future
moz 0.9
Status: REOPENED → ASSIGNED
Target Milestone: Future → mozilla0.9
moving a bunch of 0.9 bugs to 0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
What's the status on this bug? It's been pushed out forever.
*** Bug 67405 has been marked as a duplicate of this bug. ***
Marking this critical since it makes replying in the plain text editor something akin to getting a root canal.
Whiteboard: [nsbeta3-][p:3] → [nsbeta3-][p:3] critical to mozilla 0.9
This bug makes plaintext mail compose basically unusable for replies. We should make it a priority to get plaintext mail compose working, or else officially give up on it and not taunt users. (I hope we don't give up on it, though -- a lot of people prefer it and it does give a much more wysiwyg view of what you're really going to send, assuming that you convert to plaintext on send.) I'd be happy to help if Joe would give me a clue where the selection-warping code lives and how it works.
Does this have anything to do with selection? I don't use selection and I can trigger this problem.
Hell, yeah. I get bitten by this about 30 times a day.
The collapsed selection position is where the caret is and where the inserted text goes. The problem here is that the caret/selection warps to inside the pre, when it should be outside and after the pre (and usually after a blank line).
In the html dump that I put into bug #67405 you can see that the text is just included in the <pre> tag for the entire body. Is it your expectation that it look something like this: <pre> &gt;This is quoted text<br> <br> This isn't quoted and I typed it in.<br> <br> &gt;This is quoted text<br> </pre> or: <pre> &gt;This is quoted text<br> <br> </pre> This isn't quoted and I typed it in.<br> <pre> <br> &gt;This is quoted text<br> </pre>
The latter. When you put the caret into a quoted pre block and hit return, it should split the pre into two sections, and put the caret between the two pre blocks. When you type, the new text should remain between the two pre blocks, not inside either one of them. When this bug was first reported, the return did correctly split the pre, but then when you typed, the caret warped back inside the previous pre block so the newly added text showed up there. Now, however (judging by the source you posted in bug 67405) the pre block isn't even being split, so there's been a regression in the plaintext rules code for inserting a blank line. Joe?
among the words that rhyme with regression are: obsession depression recession bad impression
That's cool and all but it's not helping my editing woes. :)
Shouldn't we change the summary, adding something about text wrap to avoid dups? I would certainly look for "wrap" keyword first.
so the problem here is that we have an odd design I was not aware of when refactoring code for embedding work a while back. nsEditorShell::InstantiateEditor() can make a text editor, and html editor, or an html mail editor. It doesn't make a text mail editor. Ie, no plaintext editor ever has the mail flag lit, even if it is created for mail compose. I didn't realize the mail flag did not have a one-to-one correspondence to mail compose editors, and that assumption bit me when I refactored some code involving splitting mailcites.
ok, i have a patch. akkana, simon, can you r/sr?
Now that's refreshin' ! I'm in possession (of the patch).
Success(ion)! r=akkana, and this does seem to fix the problem. Other people who are seeing it should check to make sure it fixes it for them as well (since I think there may have been various different incarnations of this at some point). This certainly makes things better, in any case. When I bring up the plaintext reply window I see this: ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIEditorShell.GetSelectedElement]" nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)" location: "JS frame :: chrome://editor/content/editor.js :: GetObjectForProperties :: line 953" data: no] ************************************************************ An error occurred updating the cmd_objectProperties command ###!!! ASSERTION: failed to find attribute: 'attr', file nsHTMLAttributes.cpp, line 1248 but I don't think this is related to the patch at all (mentioning it just in case).
*** Bug 70575 has been marked as a duplicate of this bug. ***
This is working great for me. I haven't seen any problems with the result except for the assertion that akkana mentioned. I don't know if that was there before I added the patch or not. For the patch, sr=blizzard. GIT INTAH MUH TREE
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → FIXED
marking verified based on blizzard's comments.
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: