nsHTMLEditor::PasteAsCitedQuotation doesn't

VERIFIED FIXED in M15

Status

()

Core
Editor
P3
normal
VERIFIED FIXED
18 years ago
16 years ago

People

(Reporter: Akkana Peck, Assigned: Akkana Peck)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
It pastes the text, but not the blockquote around it.  It's not that the text
isn't inside the blockquote; it's that the blockquote node just isn't there
after we return from DeleteSelectionAndCreateNode.
(Assignee)

Updated

18 years ago
Assignee: akkana → jfrancis
Target Milestone: --- → M15
(Assignee)

Comment 1

18 years ago
Turns out this is coming from the nsAutoRules destructor.

At the end of nsEditor::CreateNode, the blockquote node has been created (with
nothing in it yet).  Then we attempt to return from CreateNode, which triggers
the nsAutoRules dtor, which deletes the newly created blockquote.

Interestingly, this doesn't happen for the pre tag inserted when pasting a
plaintext quotation.

Comment 2

18 years ago
it doesnt happen for plaintext because plaintext pretty much cant have empty 
nodes floating around (pre is the only thing we ever put into it other than text 
and breaks).  

As for html editors, this problem can be fixed by populating the blockquote 
before inserting it into the tree.  Can you use doc->CreateElement to create the 
blockquote, then populate it with the text, then InsertNode() to put it in?  then 
all should be hunky dorry.

bouncing over to Akkana for now.
Assignee: jfrancis → akkana
(Assignee)

Updated

18 years ago
Assignee: akkana → jfrancis
(Assignee)

Comment 3

18 years ago
It seems bad that we can't ever insert use the editor methods (CreateNode or
DeleteSelectionAndCreateNode) to insert a block node that doesn't already exist;
and I'm also not clear how nsEditor can tell which nodes need to have another
node inserted inside them, and which nodes can just be inserted directly, since
nsEditor isn't supposed to have knowledge of HTML.

Sending back to Joe.
(Assignee)

Comment 4

18 years ago
Maybe I can create the blockquote using non-editor techniques, and put a moz-br
inside it, then insert it using editor techniques.  I'll try that.

But I still think it's confusing that we have a method called CreateNode that
can't actually create a node (if it's a block node).  We should document that
very clearly if we're going to continue doing that (and perhaps put in an
assert, or something similar, in the case where we remove the node that was just
inserted).
(Assignee)

Comment 5

18 years ago
Oh, more detail:

I can't easily populate the new node with "the text"; what I've done in the past
is to create the blockquote, set the caret inside it, then call Paste, which
does various negotiation with the clipboard to decide whether to paste text,
html, or (eventually) images or xif or other types.  This code would have to be
factored out somehow so that it could insert either into the editor or into a
node that's not yet in the document.

I could potentially populate the new blockquote with a moz br (if that would be
enough to keep it from being removed), prior to inserting it into the document,
and rely on the rules code to remove the moz br when we inserted something more
substantial there.  I'm not sure how to create the special br without using
editor methods and without duplicating code, though.

One odd thing: InsertAsCitedQuotation doesn't have this problem; somehow the
blockquote it inserts isn't removed.  It looks like the main difference is that
InsertAsCitedQuotation includes the line:
  nsAutoRules beginRulesSniffing(this, kOpInsertQuotation, nsIEditor::eNext);
while PasteAsCitedQuotation doesn't have that.

Even better, if I insert that line at the beginning of PasteAsCitedQuotation, it
seems to solve the problem, and paste as quotation starts working again!

Joe, is this a reasonable fix?

Comment 6

18 years ago
Yes, using the autoRules wrapper around any editor action is a good idea.  That 
basically tells the editor: don't do any postprocessing until you leave this 
scope.  In turn, that allows you to call CreateNode() with an empty container, 
and not have it immediately go away (though it will still go away if you don't 
populate it before leaving the scope).

sorry if I missed this case when I put in the autoRules code.  Did the code path 
change here recently?  I'm wondering why this problem hasn't been happening for a 
long time...

marking fixed,
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 7

18 years ago
Akkana, how does Sujay verify this? I don't see the PasteAsQuotation menu item 
anymore...
(Assignee)

Comment 8

18 years ago
In the editor, Paste as Quotation has moved to the Debug menu (apparently
someone complained to Charley that we shouldn't have it in the composer
window).  In mail compose, it should be in the usual place.

But this bug isn't fixed yet; I'm waiting for the tree to open to check in the
fix.  So Sujay can't verify it yet anyway.  Reopening and assigning to myself
since I have the fix in my tree.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

18 years ago
Taking the bug.
Assignee: jfrancis → akkana
Status: REOPENED → NEW
(Assignee)

Comment 10

18 years ago
Tree just opened, so I checked in a fix.
Status: NEW → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 11

18 years ago
verified in 4/3 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.