348802 - Dynamic designMode toggling is risky/buggy

Status: RESOLVED INCOMPLETE

(Core :: DOM: Editor, defect, P5)

Description

[Peter Kasting]

designMode has a lot of issues.  From bug 348497 comment 2:

"What we should probably do is:

1)  Fix SetupEditorOnWindow to hold an nsRefPtr on the stack to the object it's 
    gonna pass around.
2)  Flush a lot earlier.  Either at the top of SetupEditorOnWindow (and add
    code to detect that this reentered and bail out of the outer invokation) or
    even in nsHTMLDocument::SetDesignMode...
3)  Add an internal API that does NOT flush for reenabling designMode, since
    flushing from inside a frame Init() method is just bad juju."

The fix for bug 348497 partially does #2.  The others are not currently being done.

Additionally, from bug 348497 comment 9:

"The testcase for bug 284245 (
https://bugzilla.mozilla.org/attachment.cgi?id=175927 ) "works" equally well
with and without this patch (I think), but there seem to be weird bugs there
either way.  For example, if you enable designmode, type a character into the
box, and then disable designmode, then you can still edit in the box--even
after going back and then forward.  This is just all screwy.  I think there are
major problems going on here.

Also, I note that nsSubDocumentFrame::ShowDocShell() isn't the only place that
twiddles designmode off and then on; nsHTMLDocument::OpenCommon() does it too.

Someone really needs to take a step back and figure out how designMode is
supposed to work, and then get it right."

Comment 1

[Peter Kasting]

Attachment: Broken branch patch for comment 0 item #3

This is a not-working-right branch patch that attempts comment 0 item #3.  By "not working right" I mean that the testcase for bug 284245 gets significantly more broken with this.  Also note that this only fixes one of the two places we twiddle designmode off then on (though this is the scarier one).

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm not sure what needs to be done here. Turning designmode on and off in https://bugzilla.mozilla.org/attachment.cgi?id=175927 seems to work fine.

Comment 3

[Peter Kasting]

I haven't been following any of this code since I filed this bug.  As you note, that "comment 9" bustage noted above is no longer present on the trunk.  Whether the underlying code here is now sane is unknown to me.  I suggest that bz comments if he knows more, since he was involved in reviewing this stuff on bug 348497; and if no one knows what the status is, that we just close this bug.

Comment 4

[Boris Zbarsky [:bzbarsky]]

We should at least think about fixing items 1-3 from comment 0.  If we decide we don't need to, we can close this bug out.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Alright, I'll leave this open for someone to think about that.

Comment 6

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority and severity.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Although I've fixed a lot around the editable state management without focus change. It seems that there are some edge cases according to fuzzing tests. However, I don't think we need this kind of meta bugs for this issue anymore.