Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11400 closed defect (fixed)

Safari: diji.tests.robot.LinkDialog test failures

Reported by: Chris Mitchell Owned by: bill
Priority: high Milestone: 1.5
Component: Editor Version: 1.4.0
Keywords: Cc: Douglas Hays
Blocked By: Blocking:

Description (last modified by bill)

GROUP "LinkDialog_tests" has 17 tests to run
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Create a new link 7321 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Create a new link with alternate target 7313 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test auto insertion of http:// for urls. 7314 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test insertion 'relative' urls. 7315 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Update existing anchor tag 7815 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Blank description invalid. 8331 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test invalid url. 8918 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test mailto url. 8915 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test mailto url prepend. 8916 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Test invalid mailto url. 8935 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Anchor Tag: Double-Click opens TooltipDialog. 3916 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Create a new image 7317 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Verify http:// is prepended 7338 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Update existing image tag 7811 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Blank description valid. 8313 ms
     _AssertFailure: doh._AssertFailure: assertTrue('false') failed: assertTrue('false') failed
     ERROR IN:
 		 function () {
							var d = new doh.Deferred();

							try{
								//Focus on the editor window
								dojo.window.scrollIntoView(editor.domNode);
								editor.focus();
								doh.robot.mouseMoveAt(editor.iframe, 500);
								doh.robot.mouseClick({left:true}, 500);

								//Find the fullscreen plugin, we'll need it.
								doh.assertTrue(ldPlugin !== null, "Verifying the link dialog.");
								var value = editor.get("value");

								doh.robot.mouseMoveAt(node, 500);
								doh.robot.mouseClick({left:true}, 500);
								doh.robot.sequence(d.getTestCallback(function(){
									var selectedElement = editor._sCall("getSelectedElement", [null]);
									doh.assertTrue(selectedElement != null);
									var tag = selectedElement.tagName? selectedElement.tagName.toLowerCase() : "";
									doh.assertTrue(tag === "img");
									doh.assertTrue(selectedElement.getAttribute("id") === "exampleImage");
								}), 2000);
							}catch(e){
								d.errback(e);
							}
							return d;
						}
 FAILED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Single click selects image. 4231 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html::LinkDialog_tests::Image Tag: Double-Click opens TooltipDialog. 3919 ms
 PASSED test: ../../dijit/tests/editor/robot/Editor_LinkDialog.html 125108 ms

Change History (9)

comment:1 Changed 9 years ago by bill

Component: DijitEditor
Description: modified (diff)
Milestone: tbd1.5

editor/robot/Editor_LinkDialog.html (the standalone test) fails for me too.

comment:2 Changed 9 years ago by Chris Mitchell

Able to reproduce on separate run. Safari Version 5.0 (6533.16), Mac OSX SnowLeopard? x64

comment:3 Changed 9 years ago by Chris Mitchell

Also able to reproduce on separate OS Safari Version 5.0 (7533.16), Windows7 x64

comment:4 Changed 9 years ago by bill

Summary: Safari5: diji.tests.robot.LinkDialog test failuresSafari: diji.tests.robot.LinkDialog test failures

Happening for me on Safari4 too.

comment:5 Changed 9 years ago by bill

Version: 1.5.0b21.4.0

This is a real bug although not a new bug: it happens in dojo 1.4 on Safari4.

It's simply that on safari, although you can insert new images, you can't edit an existing image (to change the URL or the description). The event handlers for click/doubleclick (on the <body> of the editor's <iframe>) aren't even firing. It works on chrome.

comment:6 Changed 9 years ago by bill

Cc: Douglas Hays added
Owner: set to bill
Status: newassigned

The problem seems to be this safari specific code that sets the <iframe> src attribute twice:

if(dojo.isSafari){ // Safari seems to always append iframe with src=about:blank
	setTimeout(function(){ifr.setAttribute('src', s);},0);
}

That causes the <iframe> onloadfunc to execute twice, calling onLoad() twice:

ifr._loadFunc = dojo.hitch( this, function(win){
...			
	// Do final setup and set initial contents of editor
	this.onLoad(html);
});

That might fail for a number of reasons... this specific problem is that LinkDialog registers some dojo.connect() calls to happen when the onLoadDeferred Object fires, but since a Deferred can only fire once, no dojo.connect() calls occur on the second <iframe> load.

The double-setting of ifr.src traces back to [16560], which was a checkin to fix non-safari bugs. It doesn't seem to be needed anymore; at least all the unit tests pass without it.

comment:7 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [22416]) Remove some safari workaround code that's apparently no longer needed on Safari4+. Setting iframe src twice will fire the onload handler twice, and that has problems since onLoadDeferred can only fire once. Fixes #11400 !strict.

comment:8 Changed 9 years ago by bill

(In [22419]) Fix test to correctly detect that doubleclick shows the TooltipDialog. Test was passing even when the code was broken, before [22416]. Refs #11400.

comment:9 Changed 9 years ago by bill

(In [22420]) Split tests into two groups (link dialog and image dialog). Refs #11400.

Note: See TracTickets for help on using tickets.