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)
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)
(deleted),
patch
|
Details | Diff | Splinter Review |
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:
> 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.
Reporter | ||
Comment 1•25 years ago
|
||
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.
Reporter | ||
Comment 3•25 years ago
|
||
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
Reporter | ||
Comment 6•25 years ago
|
||
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
Reporter | ||
Comment 7•25 years ago
|
||
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
Assignee | ||
Comment 8•25 years ago
|
||
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
Assignee | ||
Comment 9•25 years ago
|
||
fixed. Hitting return always gets you out of the mailquote now.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 10•24 years ago
|
||
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 → ---
Updated•24 years ago
|
Whiteboard: nsbeta3+ → {nsbeta3+}
Updated•24 years ago
|
Whiteboard: {nsbeta3+} → [nsbeta3+]
Comment 14•24 years ago
|
||
setting priority in status whiteboard (medium)
Whiteboard: [nsbeta3+] → [nsbeta3+][p;3]
Assignee | ||
Comment 15•24 years ago
|
||
correcting typo in stat whiteboard, so that i can sort my bugs by the whiteboard
Whiteboard: [nsbeta3+][p;3] → [nsbeta3+][p:3]
Comment 17•24 years ago
|
||
setting to future and adding helpwanted
Need to triage bugs to meet the glidepath for pr3
Assignee | ||
Comment 18•24 years ago
|
||
moz 0.9
Status: REOPENED → ASSIGNED
Target Milestone: Future → mozilla0.9
Assignee | ||
Comment 19•24 years ago
|
||
moving a bunch of 0.9 bugs to 0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
Comment 20•24 years ago
|
||
What's the status on this bug? It's been pushed out forever.
Comment 21•24 years ago
|
||
*** Bug 67405 has been marked as a duplicate of this bug. ***
Comment 22•24 years ago
|
||
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
Reporter | ||
Comment 23•24 years ago
|
||
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.
Comment 24•24 years ago
|
||
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.
Reporter | ||
Comment 26•24 years ago
|
||
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).
Comment 27•24 years ago
|
||
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>
>This is quoted text<br>
<br>
This isn't quoted and I typed it in.<br>
<br>
>This is quoted text<br>
</pre>
or:
<pre>
>This is quoted text<br>
<br>
</pre>
This isn't quoted and I typed it in.<br>
<pre>
<br>
>This is quoted text<br>
</pre>
Reporter | ||
Comment 28•24 years ago
|
||
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?
Assignee | ||
Comment 29•24 years ago
|
||
among the words that rhyme with regression are:
obsession
depression
recession
bad impression
Comment 30•24 years ago
|
||
That's cool and all but it's not helping my editing woes. :)
Comment 31•24 years ago
|
||
Shouldn't we change the summary, adding something about text wrap to avoid dups?
I would certainly look for "wrap" keyword first.
Assignee | ||
Comment 32•24 years ago
|
||
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.
Assignee | ||
Comment 33•24 years ago
|
||
Assignee | ||
Comment 34•24 years ago
|
||
ok, i have a patch. akkana, simon, can you r/sr?
Reporter | ||
Comment 35•24 years ago
|
||
Now that's refreshin' ! I'm in possession (of the patch).
Reporter | ||
Comment 36•24 years ago
|
||
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).
Comment 37•24 years ago
|
||
*** Bug 70575 has been marked as a duplicate of this bug. ***
Comment 38•24 years ago
|
||
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
Assignee | ||
Comment 39•24 years ago
|
||
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•