168336 - Centered text won't stay centered

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[scottputterman]

Using the 9/12 trunk build on XP.

1.  Create a new document
2.  Hit the center text button on the toolbar
3.  Hit return.
4.  Hit the left align button so you can now start typing from the left.

result:  The centered text also aligns left.

Comment 1

[sujay]

confirming...good catch.

1) launch netscape
2) launch composer
3) enter text, center it
4) hit return
5) left align

notice the text you originally entered also gets left aligned.

Comment 2

[scottputterman]

Oops.  Forgot the entering text step.  Nominating for next release.

Comment 3

[Charles Manske]

NS7.0 doesn't show this.

Comment 4

[Joe Francis]

Attachment: patch to nsHTMLEditRules.cpp

continued cleanup of GetPromotedPoint().  Removed unneeded routines
IsFirstNode()/IsLastNode().  Made GetPromotedPoint() consistently use
{node/offset} and {parent,pOffset} for doing calculations.  Inconsistent use of
local variables was cause of this bug.

Comment 5

[kinmoz]

Comment on attachment 99531 [details] [diff] [review]
patch to nsHTMLEditRules.cpp

sr=[e-mail]

==== Is there a reason why we can't call |GetNodeLocation()| inside the |while|
loop at the beginning? This will allow us to avoid an unnecessary call if
|nearNode| is initially null, and an extra unnecessary call when |nearNode|
becomes null in the loop. Looks like this same loop is in 2 places.


     // finding the real end for this point.  look up the tree for as long as
we are the 
     // last node in the container, and as long as we haven't hit the body
node.
-    if (!nsTextEditUtils::IsBody(node))
-    {
-      res = nsEditor::GetNodeLocation(node, address_of(parent), &offset);
+    res = mHTMLEditor->GetNextHTMLNode(node, offset, address_of(nearNode),
PR_TRUE);
+    if (NS_FAILED(res)) return res;
+    res = nsEditor::GetNodeLocation(node, address_of(parent), &pOffset);
+    if (NS_FAILED(res)) return res;
+    while (!nearNode && !nsTextEditUtils::IsBody(node))
+    {
+      node = parent;
+      offset = pOffset+1;  // we want to be AFTER nearNode
+      res = mHTMLEditor->GetNextHTMLNode(node, offset, address_of(nearNode),
PR_TRUE);


==== I must admit that it's a bit confusing that |GetNextHTMLNode()| and
|GetPriorHTMLNode()| require different aOffset values. For |GetNextHTMLNode()|,
aOffset is the offset of the next node you want, whereas in the
|GetPriorHTMLNode()| case, it's the offset for the node you are asking for it's
prior node. This makes the loops that use these methods look different because
in the "next" case you have to increment, and in the "prior" case you don't do
anything.

Comment 6

[Joe Francis]

Addressing Kin's comments:

1) I've altered the loops as you requested; you are absolutely right about the
unneeded call.

2) GetNext/PriorHTMLNode() are not inconsistent.  In each case the first two
params are a {node,offset} pair that specifies a location in the dom tree that
is the starting point for the search.  The incrementing of a separate offset
later in the code is due to how I am using the data provided by
GetNextHTMLNode(), not by any weirdness in the routine itself.

Comment 7

[Joe Francis]

fix landed on trunk

Comment 8

[sairuh (rarely reading bugmail)]

vrfy'd fixed with 2003.02.19 on win2k.