Closed Bug 1168079 Opened 7 years ago Closed 7 years ago

Ensure to set selection after reframing an editor

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(1 file)

While I'm working on bug 1162818, I see a lot of noisy warnings:

> [11380] WARNING: NS_ENSURE_TRUE(aSelection->RangeCount()) failed: file a:\mozilla\mc-e\src\editor\libeditor\nsEditor.cpp, line 3664
> [11380] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file a:\mozilla\mc-e\src\editor\libeditor\nsEditor.cpp, line 3643
> [11380] WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file a:\mozilla\mc-e\src\editor\libeditor\nsTextEditRules.cpp, line 437

This occurs when nsPlaintextEditor is initialized after reframed contents.
Attached patch PatchSplinter Review
By this bug, nsTextEditRules::CollapseSelectionToTrailingBRIfNeeded() doesn't run completely.
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/nsTextEditRules.cpp?rev=133ec7304f22&mark=436-437#420

It's called by AfterEdit():
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/nsTextEditRules.cpp?rev=133ec7304f22&mark=231-232#196

It's called by the destructor of nsAutoRules (via nsPlaintextEditor::EndOperation()).

An nsAutoRules during initialize is created in nsTextEditRules::CreateBogusNodeIfNeeded().
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/nsTextEditRules.cpp?rev=133ec7304f22&mark=1141-1141,1159-1159,1186-1186#1129
At line 1141, nsAutoRules is created. Then, returned from line 1159. So, selection range is not created by the end of this method (line 1186).

It's called by nsTextEditRules::Init(). And after that, if there is no selection range, it calls nsEditor::EndOfDocument() which sets caret to the end of the editor.
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/nsTextEditRules.cpp?rev=133ec7304f22&mark=129-135#111
However, before that, AfterEdit is performed.

I guess that when an editor is NOT reframed, i.e., created first time, there is no editable child. Then, CreateBogusNodeIfNeeded() runs all of them and setting caret position.

I thought where is exactly unintentional point, but I'm still not sure that. Therefore, I choose the safest fix. Before calling nsEditor::GetStartNodeAndOffset(), there must be a selection range. When there is no selection range, this patch makes CollapseSelectionToTrailingBRIfNeeded() call nsEditor::EndOfDocument() like Init().
Attachment #8610301 - Flags: review?(ehsan)
Status: NEW → ASSIGNED
Oops, I'll remove the first chunk, please ignore it.
Comment on attachment 8610301 [details] [diff] [review]
Patch

Review of attachment 8610301 [details] [diff] [review]:
-----------------------------------------------------------------

Do you have a test case for this by any chance?  That would be really nice.

::: editor/libeditor/nsTextEditRules.cpp
@@ +207,5 @@
>    {
>      NS_ENSURE_STATE(mEditor);
>      nsRefPtr<Selection> selection = mEditor->GetSelection();
>      NS_ENSURE_STATE(selection);
> +

Please leave this in.  I hate trailing whitespaces.  ;-)
Attachment #8610301 - Flags: review?(ehsan) → review+
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #3)
> Comment on attachment 8610301 [details] [diff] [review]
> Patch
> 
> Review of attachment 8610301 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do you have a test case for this by any chance?  That would be really nice.

Unfortunately, no. I've not found any symptoms when this is warned.

> ::: editor/libeditor/nsTextEditRules.cpp
> @@ +207,5 @@
> >    {
> >      NS_ENSURE_STATE(mEditor);
> >      nsRefPtr<Selection> selection = mEditor->GetSelection();
> >      NS_ENSURE_STATE(selection);
> > +
> 
> Please leave this in.  I hate trailing whitespaces.  ;-)

But I don't like to include this in this patch. I'll file a bug to removing all trailing whitespaces under editor/ with a patch instead.
https://hg.mozilla.org/mozilla-central/rev/d295209feea1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.