Fix nsEditor::DoTransaction to use transaction interfaces sanely

RESOLVED FIXED in mozilla16

Status

()

Core
Editor
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

Trunk
mozilla16
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

From nsEditor::DoTransaction:

    // We start off with an EditTxn since that's what the factory returns.
    nsRefPtr<EditTxn> editTxn = new PlaceholderTxn();

    // Then we QI to an nsIAbsorbingTransaction to get at placeholder
    // functionality
    nsCOMPtr<nsIAbsorbingTransaction> plcTxn;
    editTxn->QueryInterface(NS_GET_IID(nsIAbsorbingTransaction),
                            getter_AddRefs(plcTxn));
    // have to use line above instead of "plcTxn = do_QueryInterface(editTxn);"
    // due to our broken interface model for transactions.

    // save off weak reference to placeholder txn
    mPlaceHolderTxn = do_GetWeakReference(plcTxn);

editTxn is not used thereafter, only plcTxn.  This is outdated -- bug 489851 got rid of the factory functions that the comments refer to, so we could just use nsRefPtr<PlaceholderTxn> here, which requires no QI to be an nsIAbsorbingTransaction.  But then do_GetWeakReference doesn't work.  What's the right way to work around this?
Summary: Don't use → Fix nsEditor::DoTransaction to use transaction interfaces sanely
Hmm, how about:

nsCOMPtr<nsIAbsorbingTransaction> plcTxn = new PlaceholderTxn();

You have the concrete type here, so there's no reason to QI, and the compiler can figure things out using a static cast.
Created attachment 640058 [details] [diff] [review]
Patch

Hmm, that works.  We still need to QI it to nsITransaction, which isn't maximally elegant, but it's still a lot better.  -8 lines of comments complaining about how bad the code is!

Try: https://tbpl.mozilla.org/?tree=Try&rev=c4a58cf9f185
Attachment #640058 - Flags: review?(ehsan)
Attachment #640058 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b855bdde1a9
Flags: in-testsuite-
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/5b855bdde1a9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.