Dynamic designMode toggling is risky/buggy

NEW
Unassigned

Status

()

Core
Editor
12 years ago
10 years ago

People

(Reporter: Peter Kasting, Unassigned)

Tracking

Trunk
mozilla1.9alpha1
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
wanted-next +
blocking1.9 -
wanted1.9 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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."
(Reporter)

Comment 1

12 years ago
Created attachment 233918 [details] [diff] [review]
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).
Flags: blocking1.9?
(Reporter)

Updated

12 years ago
Depends on: 348981

Updated

12 years ago
No longer depends on: 348981
Depends on: 348981
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
QA Contact: editor
Flags: wanted1.9+
Whiteboard: [wanted-1.9]

Updated

10 years ago
Blocks: 424615
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.
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
(Reporter)

Comment 3

10 years ago
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.
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.
Alright, I'll leave this open for someone to think about that.
You need to log in before you can comment on or make changes to this bug.