It seems that EditorBase::EndPlaceholderTransaction() doesn't need to retrieve nor hold nsCaret anymore

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Blocks 1 bug)

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/editor/libeditor/EditorBase.cpp#987,1004-1021

> EditorBase::EndPlaceholderTransaction()
> {
>   MOZ_ASSERT(mPlaceholderBatch > 0,
>              "zero or negative placeholder batch count when ending batch!");
>   if (mPlaceholderBatch == 1) {
>     RefPtr<Selection> selection = GetSelection();
> 
>     // By making the assumption that no reflow happens during the calls
>     // to EndUpdateViewBatch and ScrollSelectionIntoView, we are able to
>     // allow the selection to cache a frame offset which is used by the
>     // caret drawing code. We only enable this cache here; at other times,
>     // we have no way to know whether reflow invalidates it
>     // See bugs 35296 and 199412.
>     if (selection) {
>       selection->SetCanCacheFrameOffset(true);
>     }
> 
>     {
>       // Hide the caret here to avoid hiding it twice, once in EndUpdateViewBatch
>       // and once in ScrollSelectionIntoView.
>       RefPtr<nsCaret> caret;
>       nsCOMPtr<nsIPresShell> presShell = GetPresShell();
> 
>       if (presShell) {
>         caret = presShell->GetCaret();
>       }
> 
>       // time to turn off the batch
>       EndUpdateViewBatch();
>       // make sure selection is in view
> 
>       // After ScrollSelectionIntoView(), the pending notifications might be
>       // flushed and PresShell/PresContext/Frames may be dead. See bug 418470.
>       ScrollSelectionIntoView(false);
>     }

EditorBase::EndPlaceholderTransation() retrieves PresShell and nsCaret, and hold them until calling ScrollSelectionIntoView(). This was added by bug 240933 with |StCaretHider caretHider(caret);|.
https://bugzilla.mozilla.org/attachment.cgi?id=459123&action=diff

However, this was removed by bug 805697. Looks like that this is mistake of bug 805697.
https://bugzilla.mozilla.org/attachment.cgi?id=675404&action=diff
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8902738 [details]
Bug 1395157 - Make EditorBase::EndPlaceholderTransaction() not retrieve nor hold nsIPresShell and nsCaret

https://reviewboard.mozilla.org/r/174386/#review179862
Attachment #8902738 - Flags: review?(ehsan) → review+

Comment 4

2 years ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ea2850222e1b
Make EditorBase::EndPlaceholderTransaction() not retrieve nor hold nsIPresShell and nsCaret r=Ehsan
https://hg.mozilla.org/mozilla-central/rev/ea2850222e1b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.