Closed
Bug 781763
Opened 12 years ago
Closed 12 years ago
Firefox inconsistently wraps the child elements in div with contentEditable="true" and in an iframe
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 746515
People
(Reporter: iliyan.peychev, Unassigned)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.57 Safari/537.1
Steps to reproduce:
I created a div element with contentEditable = "true":
<div contenteditable="true" class="editor" id="editor">
Testing<br>
<span>selectall<br></span>
</div>
and then executed the following commands:
document.execCommand('styleWithCSS', null, 1);
document.execCommand('insertbronreturn', false, false);
document.execCommand('selectall', null);
document.execCommand('fontname', null, 'yui-tmp');
Actual results:
Firefox (14.0.1) set "style" attribute with "font-family: yui-tmp;" to the element and left the content untouched:
<div style="font-family: yui-tmp;" class="editor" id="editor" contenteditable="true">
Testing<br>
<span>selectall<br></span>
</div>
Expected results:
Firefox should wrap the child elements in this way:
<div contenteditable="true" class="editor" id="editor">
<span style="font-family: yui-tmp;"> Testing<br></span>
<span>
<span style="font-family: yui-tmp;">selectall</span><br>
</span>
</div>
Firefox does the above but if I use an iframe instead this div element.
In more words - there is inconsistency, it produces different content depending on the element I'm using as an editor.
I tested what the other browsers do and here are the results:
All the other main browsers do the things consistently. It does not matter if I use an iframe or div element, they all wrap the elements inside:
Chome 21.0 wraps them with <span style="font-family: yui-tmp;">
IE8 and IE9 wrap them with <font face="yui-tmp">
If I use iframe, Firefox 14.01 wraps them with <span style="font-family: yui-tmp;">
If I use div with contentEditable as in the example above, it does not wrap the children, it adds style="font-family: yui-tmp;" to the editor itself.
I'm providing test page as an attachment.
|
Reporter | |
Updated•12 years ago
|
Component: Untriaged → Editor
Product: Firefox → Core
|
||
Updated•12 years ago
|
Attachment #650843 -
Attachment mime type: text/plain → text/html
|
|
Reporter | |
Comment 1•12 years ago
|
||
|
|
Reporter | |
Updated•12 years ago
|
Attachment #651130 -
Attachment mime type: text/plain → text/html
|
|
Reporter | |
Updated•12 years ago
|
Attachment #651130 -
Attachment description: Test page which confirms the issue (executes the same command but in an iframe) → Test page which confirms the issue (executes the same command but in an iframe)
Run this page from your computer, because (at least Chrome) it will not run - YUI seed file cannot be loaded from HTTPS.
|
|
Reporter | |
Updated•12 years ago
|
Attachment #651130 -
Attachment description: Test page which confirms the issue (executes the same command but in an iframe)
Run this page from your computer, because (at least Chrome) it will not run - YUI seed file cannot be loaded from HTTPS. → Test page which confirms the issue (executes the same command but in an iframe)
Run this page from your computer, because (at least in Chrome) it will not run - YUI seed file cannot be loaded from HTTPS.
|
|
Reporter | |
Comment 2•12 years ago
|
||
The error is in editor/libeditor/html/nsHTMLEditorStyle.cpp
In this function:
nsHTMLEditor::SetInlineProperty
we are invoking PromoteInlineRange function:
http://hg.mozilla.org/releases/mozilla-release/file/cedd103f2438/editor/libeditor/html/nsHTMLEditorStyle.cpp#l165
Note this loop in PromoteInlineRange function:
http://hg.mozilla.org/releases/mozilla-release/file/cedd103f2438/editor/libeditor/html/nsHTMLEditorStyle.cpp#l887
while ( startNode &&
!nsTextEditUtils::IsBody(startNode) &&
IsEditable(startNode) &&
IsAtFrontOfNode(startNode, startOffset) )
And especially this line:
!nsTextEditUtils::IsBody(startNode)
http://hg.mozilla.org/releases/mozilla-release/file/cedd103f2438/editor/libeditor/html/nsHTMLEditorStyle.cpp#l888
In case of iframe this is true so the body of the iframe is not being included in range.
But when we have element with contentEditable = true, this is false, so startNode becomes the parent (aka the editor itself).
Then, when we return in SetInlineProperty function, the number of nodes is 1, but in case of iframe it is 3:
http://hg.mozilla.org/releases/mozilla-release/file/cedd103f2438/editor/libeditor/html/nsHTMLEditorStyle.cpp#l240
|
||
Comment 3•12 years ago
|
||
Thanks for the report, and all the debugging! I can reproduce the problem in Firefox 14, and I believe this is basically the same thing as bug 746515, which was fixed in Firefox 15. In Firefox 15 or higher, we try to never add style="" to any element other than a <span> with no attributes: instead we wrap its children in a <span>. I tested in Firefox 17 and your test-case works for me.
Could you download and test in a nightly <http://nightly.mozilla.org/>, and see if you agree that the problem is fixed? If so, I'll mark this a duplicate of bug 746515. Otherwise, please provide another test-case that demonstrates that a problem still exists. Thank you!
|
|
Reporter | |
Comment 4•12 years ago
|
||
With Firefox nightly the results are as follow:
1) without iframe:
<div class="editor" id="editor" contenteditable="true">
<span style="font-family: yui-tmp;">
Testing<br>
<span>selectall<br></span>
</span>
</div>
2) with iframe:
<body id="yui_3_6_0_2_1344848097112_37" style="display: block;">
<font face="yui-tmp">
Testing<br>
<span>selectall<br></span>
</font>
</body>
It seems Firefox nightly does not set anymore style to the parent element in case of contentEditable=true
However, the results are still different than these in Chrome, IE8 and IE9 - all they wrap both
"Testing<br>" and "selectall" in nodes (span or font).
For example in Chrome:
<div contenteditable="true" class="editor" id="editor">
<span style="font-family: yui-tmp;">
Testing<br>
</span>
<span>
<span style="font-family: yui-tmp;">selectall</span><br>
</span>
</div>
In order to test I used the same pages already attached to this issue.
Thanks,
|
|
||
Comment 5•12 years ago
|
||
(In reply to iliyan.peychev from comment #4)
> However, the results are still different than these in Chrome, IE8 and IE9 -
> all they wrap both
> "Testing<br>" and "selectall" in nodes (span or font).
>
> For example in Chrome:
>
> <div contenteditable="true" class="editor" id="editor">
> <span style="font-family: yui-tmp;">
> Testing<br>
> </span>
> <span>
> <span style="font-family: yui-tmp;">selectall</span><br>
> </span>
> </div>
Is this a problem with Firefox, or IE/Chrome? I believe the spec currently supports Firefox on this one. If this is a problem, we could change the spec and Firefox, but only if it's really causing issues for you.
|
|
Reporter | |
Comment 6•12 years ago
|
||
The previous versions of Firefox produced the same results as Chrome and IE. Starting from Firefox 15 the things has changed so I would say we have some inconsistency here.
For example - now when we wrap the selection with div elements we will have different results in Firefox and all the other main browsers.
|
|
||
Comment 7•12 years ago
|
||
(In reply to iliyan.peychev from comment #6)
> The previous versions of Firefox produced the same results as Chrome and IE.
> Starting from Firefox 15 the things has changed so I would say we have some
> inconsistency here.
>
> For example - now when we wrap the selection with div elements we will have
> different results in Firefox and all the other main browsers.
Unfortunately, everything relating to contenteditable/execCommand() is inconsistent. Just because other browsers do things differently doesn't mean we should follow the majority. You have to give specific reasons why Firefox's behavior is not just *different*, but *worse*. There's a specification, and if browsers ever start to follow the specification more then there will be convergence, but just trying to follow what most other browsers do is not a good idea here.
In this specific case, I think Firefox's behavior is better, because it produces fewer elements. The Google employees I've spoken with have emphasized that producing shorter markup from execCommand() is a goal of theirs, and there's plenty of code and spec wording in execCommand() whose sole purpose is to make the output markup shorter. So I think this is an IE/Chrome bug, not a Firefox/spec bug.
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•