164054 - Place selection at first visible content on editor doc load

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Joe Francis]

This is to partially hide bug 98619.  I need to place the selection at doc load
time at the first editable content.

Comment 1

[Joe Francis]

Attachment: patch to editor

Well, this started out as a really simple patch to make
nsHTMLEditor::CollapseSelectionToStart() simply call
nsEditor::BeginningOfDocument().  That fixed the normal document load case for
most docs.

Then I noticed switching back from source mode still didn't set selection
properly.  Turned out there were two reasons.  1)
nsEditor::BeginningOfDocument() lacks the sophistication to really tell
formatting text nodes from visible text nodes.	2) When switch from source
view, the composer commands js was explicitly setting the selection to {body,0}
after nsHTMLEditor::CollapseSelectionToStart() had done' it's thing.

So the js cal to set selection is gone, BegginingOfDocument() is now overridden
in nsHTMLEditor where it can be smarter, CollapseSelectionToStart() calls it,
and for giggles I shortened nsEditor::BegginingOfDocument() because it was such
a mess.  It's semantics are unchanged.

I also had to initialize the dtd in nsHTMLEditor::Init() a little earlier so
that it would be around in time for the whitespace code to use in
nsHTMLEditor::BegginingOfDocument().

Comment 2

[Kathleen :Brade]

Comment on attachment 101526 [details] [diff] [review]
patch to editor

r=brade

Comment 3

[kinmoz]

Comment on attachment 101526 [details] [diff] [review]
patch to editor

sr=[e-mail]


==== I see different error check styles in nsEditor and
nsHTMLEditor::BeginningOfDocument():

+  if (NS_FAILED(res))
+    return res;
+  if (!selection)
+    return NS_ERROR_NOT_INITIALIZED;

...

+  if (NS_FAILED(res)) return res; 
+  if (!rootElement)   return NS_ERROR_NULL_POINTER; 

...

+      if (NS_FAILED(result)) { return result; }
+      if (!parentNode) { return NS_ERROR_NULL_POINTER; }


==== In |nsHTMLEditor::BeginningOfDocument()| should we initialize |selNode|
and |selOffset| to (body, 0)? My concern is if it's possible to go through the
while loop and keep hitting |visType==nsWSRunObject::eOtherBlock| until we exit
the loop?


==== Just curious, why do we have |CollapseSelectionToStart()| and
|BeginningOfDocument()|:


    nsEditor::BeginningOfDocument()
    nsPlaintextEditor::CollapseSelectionToStart()
    nsHTMLEditor::BeginningOfDocument()
    nsHTMLEditor::CollapseSelectionToStart()

Comment 4

[Joe Francis]

Kin, I don't know why we have the two different routines.  I think Charlie added 
CollapseSelectionToStart(), and it used to call some code that would not look 
inside tables.  Perhaps he wanted that distinction in behavior.  

Or perhaps he didn't notice BeginningOfDocument(), which is an odd name anyway.  
I wouldn't guess that would touch my selection from the name.

Comment 5

[Joe Francis]

kin: you can't exit the loop untikl done==PR_TRUE, and that only happens where
selNode is set.

Comment 6

[Joe Francis]

fixed on trunk

Comment 7

[Aleksey Nogin]

This commit have added a "might be used uninitialized" warning on Brad TBox:

+editor/libeditor/html/nsHTMLEditor.cpp:494
+ `PRInt32 selOffset' might be used uninitialized in this function

While this appears to be a case of compiler not being smart enough, it would be
nice to have fewer warnings anyway (especially since these "might be used
uninitialized" warnings occasionally do indicate nasty bugs) .

P.S. See bug 59652 for more on "might be used uninitialized" warnings.

Comment 8

[sairuh (rarely reading bugmail)]

rs vrfy.