Closed Bug 407127 Opened 13 years ago Closed 10 years ago

ContentEditable Iframe Content gets uneditable if iframe is position style is changed

Categories

(Core :: DOM: Editor, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: u295173, Assigned: ehsan)

References

()

Details

(Keywords: dev-doc-complete, testcase)

Attachments

(7 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1

Iframe with ContentDitale=true in the content.
If the Container of the Iframe is set to position: absolute, the iframes content gets unebditable.

Reproducible: Always

Steps to Reproduce:
1. test it with the file in the url
Actual Results:  
the iframes content isnt editable anymore.
contentEditable = true wont make it editable.
only
contentEditable = false
contentEditable = true
will do the job

Expected Results:  
the iframes content should stay editable
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120609 Minefield/3.0b2pre

There is some improvement since Firefox 2.
It still shows no caret, you can add letters but not on the place you want, "backspace" does something but not "delete". So "editing" is still a too big word.
This is confirmed.

Almost one year after the bug report, this still remains an issue and is easily reproducable by adding position: absolute to an iframe.
Taking popular frameworks like ExtJS into account, this can become quite a straining bug (at least it is to us).
Somebody take the afford of adding a simple style rule to an iframe, and fix this bug please.
Attached file Test case (obsolete) —
Some one remove the testcase I field please. I mistakenly attached it to the wrong bug!
Attachment #457795 - Attachment is obsolete: true
Attached file Test case
Keywords: testcase
This happens because the GetCaret call in nsEditor::InitializeSelection fails: <
Assignee: nobody → ehsan
(In reply to comment #6)
> This happens because the GetCaret call in nsEditor::InitializeSelection fails:
> <

http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.cpp#5169
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [post-2.0]
And... here's why this happens.

Upon initializing the editor, we go ahead and store a weak pointer to the presshell in mPresShellWeak, and assume that the presshell is alive as long as we can grab a strong reference off of the weak pointer.

But that assumption is false.  What happens in reality is that we get a pointer to a presshell which is Destroy()'ed, but still exists in memory, and we try to use it, which leads to bad things happening.

This is very broken.  We need to stop caching the presshell pointer, and just read it off of the document when we need to.  Patches coming up.
This is a refactoring change to make the main fix in this bug more elegant.
Attachment #518204 - Flags: review?(roc)
Comment on attachment 518204 [details] [diff] [review]
Part 1: Stop using mPresShellWeak directly everywhere

GetPresShell needs to be deCOMtaminated!
Attachment #518204 - Flags: review?(roc) → review+
(In reply to comment #10)
> Comment on attachment 518204 [details] [diff] [review]
> Part 1: Stop using mPresShellWeak directly everywhere
> 
> GetPresShell needs to be deCOMtaminated!

Yes, in a follow-up though!  :-)
And for that matter, much of the editor code needs to be deCOMtaminated!
After some more investigation, there seems to be a lot more going on here.  mSelConWeak will point to the old presshell for HTML editors (text editors have their own selection controller implementation), so this patch refactors the editor to prepare it for not using that member variable directly.
Attachment #518410 - Flags: review?(roc)
Some places in the editor code use the presshell just to get the document.  This is wrong, because that would fail if the document doesn't have a presshell, which causes things like <http://mxr.mozilla.org/mozilla-central/source/layout/forms/test/test_bug287446.html?force=1> to fail.
Attachment #518606 - Flags: review?(roc)
Duplicate of this bug: 467333
Blocks: 639695
Depends on: post2.0
Blocks: 644513
Duplicate of this bug: 646064
Depends on: 650083
Updated documentation:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/NsIEditor

(note that this article in general needs tons of work, but this particular issue has been addressed).

Also mentioned on Firefox 5 for developers.
(In reply to comment #21)
> Also mentioned on Firefox 5 for developers.

Do you object to me removing that entry from that page?  nsIEditor is not really a useful thing for external code to rely on, and I don't want to draw needless attention to it.  :)
Oh, yeah, go ahead if you like.
(In reply to comment #23)
> Oh, yeah, go ahead if you like.

Done, thanks!
Duplicate of this bug: 504268
You need to log in before you can comment on or make changes to this bug.