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

RESOLVED FIXED in mozilla5

Status

()

defect
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: u295173, Assigned: Ehsan)

Tracking

({dev-doc-complete, testcase})

unspecified
mozilla5
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(7 attachments, 1 obsolete attachment)

Reporter

Description

12 years ago
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.

Comment 2

11 years ago
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

9 years ago
Posted file Test case (obsolete) —

Comment 4

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

Updated

9 years ago
Attachment #457795 - Attachment is obsolete: true
Assignee

Comment 5

8 years ago
Posted file Test case
Assignee

Updated

8 years ago
Keywords: testcase
Assignee

Comment 6

8 years ago
This happens because the GetCaret call in nsEditor::InitializeSelection fails: <
Assignee: nobody → ehsan
Assignee

Comment 7

8 years ago
(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]
Assignee

Comment 8

8 years ago
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.
Assignee

Comment 9

8 years ago
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+
Assignee

Comment 11

8 years ago
(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!  :-)
Assignee

Comment 12

8 years ago
And for that matter, much of the editor code needs to be deCOMtaminated!
Assignee

Comment 13

8 years ago
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)
Assignee

Comment 15

8 years ago
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)
Assignee

Updated

8 years ago
Duplicate of this bug: 467333
Assignee

Updated

8 years ago
Blocks: 639695
Assignee

Updated

8 years ago
Depends on: post2.0

Updated

8 years ago
Blocks: 644513
Assignee

Updated

8 years ago
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.
Assignee

Comment 22

8 years ago
(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.
Assignee

Comment 24

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

Done, thanks!
Assignee

Updated

8 years ago
Duplicate of this bug: 504268
You need to log in before you can comment on or make changes to this bug.