Ticket #455 (closed defect: fixed)

Opened 8 years ago

Last modified 8 years ago

Javascript Error on inserting new Images

Reported by: Christian Nobis Owned by: yermol
Priority: normal Milestone:
Component: Plugin_ImageManager Version: trunk
Severity: normal Keywords: Plugin Manager img.style javascript error
Cc:

Description (last modified by mokhet) (diff)

Having an empty document and inserting an image using the Plugin ImageManager causes the image to be inserted whithout style information (no border and margin settings are inserted) , an javascript-error and the ImageManager-window not to close. 

Error: img.style has no properties
File: http://host/xinha/plugins/ImageManager/image-manager.js
Line: 198

When inserting an Image (having this error), giving properties to that image (using the image-properties-dialog) and inserting the same image again sometimes this error disappears.

I'm using Mozilla 1.7.8 but also tested in Firefox 1.0. No problems when using IE.

Thanks

Christian Nobis

Change History

Changed 8 years ago by anonymous

  • summary changed from Javascript Error un inserting new Images to Javascript Error on inserting new Images

Changed 8 years ago by mokhet 

var img = image;
if (!img) {
  var sel = editor._getSelection();
  var range = editor._createRange(sel);
  editor._doc.execCommand("insertimage", false, param.f_url);
  if (HTMLArea.is_ie) {
    img = range.parentElement();
    // wonder if this works...
    if (img.tagName.toLowerCase() != "img") {
      img = img.previousSibling;
    }
  } else {
    img = range.startContainer.previousSibling; // HERE
  }
} else {
  img.src = param.f_url;
}

If the selection is positioned right after another image or in an empty document
we are running : img = range.startContainer.previousSibling;

Then typeof img.style is undefined and the bug occurs.

Changed 8 years ago by mokhet

even worse, when an image is inserted right adjacent to another (imgX), then the styles are applied to this image (imgX) instead of the new inserted one.

Changed 8 years ago by mokhet

I'm using venkman 0.9.85 (with FF 1.0.4) to get infos while running, i added a breakpoing at 

 img = range.startContainer.previousSibling;

test case 1 : empty text, nothing in the editor, not even a <p />

range.startContainer.previousSibling = {text}
range.startContainer.previousSibling.nodeName = 'BODY'
range.startContainer.previousSibling.nodeType = 3
no style object is present

range.startContainer.firstChild = {HTMLImageElement}
range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'the name of the image we inserted'
style object is found

the image is also in range.startContainer.lastChild

test case 2 : text = '<p>FooBar?</p>', cursor between o and B, nothing selected

range.startContainer.previousSibling = {HTMLImageElement}
range.startContainer.previousSibling.nodeName = 'IMG'
range.startContainer.previousSibling.nodeType = 1
range.startContainer.previousSibling.src = 'the name of the image we inserted'
style object is found

test case 3 : text = '<p>FooBar?</p>', cursor before F, nothing selected

range.startContainer.previousSibling = {HTMLImageElement}
range.startContainer.previousSibling.nodeName = 'IMG'
range.startContainer.previousSibling.nodeType = 1
range.startContainer.previousSibling.src = 'the name of the image we inserted'
style object is found

test case 4 : text = '<p>FooBar?</p>', cursor after r, nothing selected

range.startContainer.nextSibling = {HTMLImageElement}
range.startContainer.nextSibling.nodeName = 'IMG'
range.startContainer.nextSibling.nodeType = 1
range.startContainer.nextSibling.src = 'the name of the image we inserted'
style object is found

test case 5 : text = '<p>FooBar?</p>', selection is ooB

range.startContainer.previousSibling = {HTMLImageElement}
range.startContainer.previousSibling.nodeName = 'IMG'
range.startContainer.previousSibling.nodeType = 1
range.startContainer.previousSibling.src = 'the name of the image we inserted'
style object is found

test case 6 : text = '<p>FooBar?</p>', selection is FooBar?

range.startContainer.firstChild = {HTMLImageElement}
range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'the name of the image we inserted'
style object is found

test case 7 : text = '<img src="xxx">', cursor before the image, nothing selected

range.startContainer.previousSibling = {text}
range.startContainer.previousSibling.nodeName = '#text'
range.startContainer.previousSibling.nodeType = 3
no style object is present

range.startContainer.firstChild = {HTMLImageElement}
range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'the name of the image we inserted'
style object is found

test case 8 : text = '<img src="xxx">', cursor after the image, nothing selected

range.startContainer.previousSibling = {text}
range.startContainer.previousSibling.nodeName = '#text'
range.startContainer.previousSibling.nodeType = 3
no style object is present

range.startContainer.firstChild = {HTMLImageElement}
range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'xxx' . So this is not the image we inserted
style object is found

range.startContainer.lastChild = {HTMLImageElement}
range.startContainer.lastChild.nodeName = 'IMG'
range.startContainer.lastChild.nodeType = 1
range.startContainer.lastChild.src = 'the name of the image we inserted'
style object is found

test case 9 : text = '<p><img src="xxx"></p>', cursor before the image, nothing selected

range.startContainer.firstChild = {HTMLImageElement}
range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'the name of the image we inserted'
style object is found

test case 10 : text = '<p><img src="xxx"></p>', cursor after the image, nothing selected

range.startContainer.firstChild.nodeName = 'IMG'
range.startContainer.firstChild.nodeType = 1
range.startContainer.firstChild.src = 'xxx' . So this is not the image we inserted
style object is found

range.startContainer.lastChild = {HTMLImageElement}
range.startContainer.lastChild.nodeName = 'IMG'
range.startContainer.lastChild.nodeType = 1
range.startContainer.lastChild.src = 'the name of the image we inserted'
style object is found

test case 11 : text = '<p>FooBar?<img src="xxx"></p>', cursor between r and the image, nothing selected

range.startContainer.nextSibling = {HTMLImageElement}
range.startContainer.nextSibling.nodeName = 'IMG'
range.startContainer.nextSibling.nodeType = 1
range.startContainer.nextSibling.src = 'the name of the image we inserted'
style object is found

test case 12 : text = '<p>FooBar?<img src="xxx"></p>', selection is Bar<img>

range.startContainer.nextSibling = {HTMLImageElement}
range.startContainer.nextSibling.nodeName = 'IMG'
range.startContainer.nextSibling.nodeType = 1
range.startContainer.nextSibling.src = 'the name of the image we inserted'
style object is found

test case 13 : text = '<p>Foo<img src="xxx">Bar</p>', cursor between o and the image, nothing selected

range.startContainer.nextSibling = {HTMLImageElement}
range.startContainer.nextSibling.nodeName = 'IMG'
range.startContainer.nextSibling.nodeType = 1
range.startContainer.nextSibling.src = 'the name of the image we inserted'
style object is found

test case 14 : text = '<p>Foo<img src="xxx">Bar</p>', cursor after the image and before the B, nothing selected

range.startContainer.previousSibling = {HTMLImageElement}
range.startContainer.previousSibling.nodeName = 'IMG'
range.startContainer.previousSibling.nodeType = 1
range.startContainer.previousSibling.src = 'the name of the image we inserted'
style object is found

test case 15 : text = '<p>Foo<img src="xxx">Bar</p>', selection is <img>Bar

range.startContainer.previousSibling = null

range.startContainer.nextSibling = null

range.startContainer.firstChild = {text}
range.startContainer.firstChild.nodeName = '#text'
range.startContainer.firstChild.nodeType = 3
no style object is present

range.startContainer.lastChild = {text}
range.startContainer.lastChild.nodeName = '#text'
range.startContainer.lastChild.nodeType = 3
no style object is present

but where can be our image in this case ? it is in range.startContainer.firstChild.nextSibling :(

I guess there is many more different case i cant think of right now, but it seems obvious that our inserted image is moving in the range.startContainer object

* 1 : range.startContainer.firstChild
* 2 : range.startContainer.previousSibling
* 3 : range.startContainer.previousSibling
* 4 : range.startContainer.nextSibling
* 5 : range.startContainer.previousSibling
* 6 : range.startContainer.firstChild
* 7 : range.startContainer.firstChild
* 8 : range.startContainer.lastChild
* 9 : range.startContainer.firstChild
* 10 : range.startContainer.lastChild
* 11 : range.startContainer.nextSibling
* 12 : range.startContainer.nextSibling
* 13 : range.startContainer.nextSibling
* 14 : range.startContainer.previousSibling
* 15 : range.startContainer.firstChild.nextSibling

the content of the cells in this table is supposed to show the nodeType or null because i think the nodeType got to be checked in some way, and a [X] if the object is the image we looking for (the one we just inserted)

Test case previousSibling nextSibling firstChild lastChild
1 3 null [X] 1 [X] 1
2 [X] 1 null null null
3 [X] 1 null null null
4 null [X] 1 null null
5 [X] 1 null null null
6 null null [X] 1 [X] 1
7 3 null [X] 1 1
8 3 null 1 [X] 1
9 null null [X] 1 1
10 null null 1 [X] 1
11 null [X] 1 null null
12 null [X] 1 null null
13 null [X] 1 null null
14 [X] 1 null null null
15 null null 3 3

this range thing is way out of my comprehension but I hope it helps someone to find out what's going on.

Changed 8 years ago by mokhet

  • description modified (diff)

using this patch seems to fix the problem for me so far. 

Index: image-manager.js
===================================================================
--- image-manager.js	(revision 307)
+++ image-manager.js	(working copy)
@@ -165,19 +165,21 @@
 		}
 		var img = image;
 		if (!img) {
-			var sel = editor._getSelection();
-			var range = editor._createRange(sel);			
-			editor._doc.execCommand("insertimage", false, param.f_url);
 			if (HTMLArea.is_ie) {
+        var sel = editor._getSelection();
+        var range = editor._createRange(sel);
+        editor._doc.execCommand("insertimage", false, param.f_url);
 				img = range.parentElement();
 				// wonder if this works...
 				if (img.tagName.toLowerCase() != "img") {
 					img = img.previousSibling;
 				}
 			} else {
-				img = range.startContainer.previousSibling;
+        img = document.createElement('img');
+        img.src = param.f_url;
+        editor.insertNodeAtSelection(img);
 			}
-		} else {			
+		} else {
 			img.src = param.f_url;
 		}
 		
@@ -209,4 +211,3 @@
 	}, outparam);
 };
 
-

Changed 8 years ago by Wil Clouser <wil.clouser@…>

mokhet: That patched fixed up my problem as well. I was getting the same javascript errors when inserting an image directly before a <br />.

And for anyone else reading this, isn't just an ImageManager plugin issue. If you're still using the original image dialog, the code is in htmlarea.js (but the same patch should work).

Changed 8 years ago by mokhet

Bump.

One month later, this patch is working great for all my gecko users, none of them has complained again about this bug. For me it looks ok to apply in ImageManager plugin and in the Xinha core _insertImage function, but as I stated previously my understanding of the range object is too poor to be sure this patch doesnt break something specific Xinha developers wanted.

Should this patch applied in ImageManager plugin ?
Should this patch applied in _insertImage core function ?
If no, how can we fix the bug ?

Changed 8 years ago by gogo

  • status changed from new to closed
  • resolution set to fixed

Applied in changeset:408 (to both ImageManager and core)

Note: See TracTickets for help on using tickets.