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

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[u295173]

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

Comment 1

[Ria Klaassen (not reading all bugmail)]

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.

Comment 2

[Robin casey]

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.

Comment 3

[Veera]

Attachment: Test case

Comment 4

[Veera]

Some one remove the testcase I field please. I mistakenly attached it to the wrong bug!

Comment 5

[(no longer active)]

Attachment: Test case

Comment 6

[(no longer active)]

This happens because the GetCaret call in nsEditor::InitializeSelection fails: <

Comment 7

[(no longer active)]

(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

Comment 8

[(no longer active)]

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.

Comment 9

[(no longer active)]

Attachment: Part 1: Stop using mPresShellWeak directly everywhere

This is a refactoring change to make the main fix in this bug more elegant.

Comment 10

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

Comment on attachment 518204 [details] [diff] [review]
Part 1: Stop using mPresShellWeak directly everywhere

GetPresShell needs to be deCOMtaminated!

Comment 11

[(no longer active)]

(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!  :-)

Comment 12

[(no longer active)]

And for that matter, much of the editor code needs to be deCOMtaminated!

Comment 13

[(no longer active)]

Attachment: Part 2: Stop using mSelConWeak everywhere

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.

Comment 14

[(no longer active)]

Attachment: Part 3: Get the selection controller for HTML editors from the presshell, instead of holding on to a presshell reference masked as a selection controller

Comment 15

[(no longer active)]

Attachment: Part 4: Avoid using the presshell needlessly because it may not always be available

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.

Comment 16

[(no longer active)]

Attachment: Part 5: Fix the editor to not rely on cached presshells and selection controllers, because they may not always be valid

Here is the main fix and the test.

Comment 17

[(no longer active)]

Attachment: Part 6: Remove the presshell argument to nsIEditor::Init as it's no longer necessary

Comment 19

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/5e5a0939aa37
http://hg.mozilla.org/mozilla-central/rev/5f4b45bf77e2
http://hg.mozilla.org/mozilla-central/rev/b8f7b7bfb440
http://hg.mozilla.org/mozilla-central/rev/81c3902d4e78
http://hg.mozilla.org/mozilla-central/rev/1d37d515d433
http://hg.mozilla.org/mozilla-central/rev/820c0e4e88db

Comment 21

[Eric Shepherd [:sheppy]]

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.

Comment 22

[(no longer active)]

(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.  :)

Comment 23

[Eric Shepherd [:sheppy]]

Oh, yeah, go ahead if you like.

Comment 24

[(no longer active)]

(In reply to comment #23)
> Oh, yeah, go ahead if you like.

Done, thanks!