Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16286 closed defect (fixed)

dojox/editor/plugins/TablePlugins "Insert Table Row After" and others fail (IE9)

Reported by: HaraldOnline Owned by: bill
Priority: undecided Milestone: 1.7.5
Component: Editor Version: 1.8.1
Keywords: Cc: Jared Jurkiewicz, Mike Wilcox, cjolif, Ed Chatelain
Blocked By: Blocking:

Description

When using the dijit/Editor together with the dojox/editor/plugins/TablePlugins I receive a JS error when trying to expand a table using "insert Table Row After" (similar errors with other buttons):

insertRow property null or undefined (error message was in German so please bear with me)

  case "insertTableRowAfter":
     r = o.tbl.insertRow(o.trIndex+1);

The cause of the problem is that o.tbl is null. At least for IE9 I fixed the problem adding this.editor.focus(); to getTableInfo:

getTableInfo: function(forceNewData){
		// summary:
		//		Gets the table in focus
		//		Collects info on the table - see return params
		//
		if(forceNewData){ this._tempStoreTableData(false); }
		if(this.tableData){
			// tableData is set for a short amount of time, so that all
			// plugins get the same return without doing the method over
			//console.log("returning current tableData:", this.tableData);
			return this.tableData;
		}
		var tr, trs, td, tds, tbl, cols, tdIndex, trIndex;


	    // MUE 31.10.2012 added this.editor.focus();
		this.editor.focus();

		td = this.editor.getAncestorElement("td");
		if(td){ tr = td.parentNode; }

            [...]

Change History (15)

comment:1 Changed 7 years ago by Adam Peller

Cc: Jared Jurkiewicz Mike Wilcox cjolif added

comment:2 Changed 7 years ago by bill

Component: DojoxEditor

I think Mike owns (i.e. originally wrote) the TablePlugin code.

comment:3 Changed 7 years ago by Mike Wilcox

Originally, yes, but Jared took it over and made it actually work :)

comment:4 Changed 7 years ago by Ed Chatelain

I tried adding the call to this.editor.focus(); inside a check for isIE in getTableInfo. While it did seem to fix the problem for most of the buttons, it seems to cause a problem for setting the "Background Color Table Cell" on IE9 and IE10.

IE7 and IE8 seem to be having trouble setting the "Background Color Table Cell" with or without the change.

comment:5 Changed 7 years ago by bill

OK, well the problem setting the background color is complicated.

It starts when the user mouse-downs on the DropDownButton in the Toolbar. That calls onDisplayChanged() where withinTable==false, causing all the table related buttons to be disabled. This is different than other browsers where the buttons stay enabled. So, this is the first bug, and I suspect it's related to the original issue in this ticket about o.tbl being undefined on IE9.

Anyway, despite that bug, the drop down still opens, and the user, after picking a color, mouse-down's on the "Set" button. This triggers the focus manager (dijit/focus) to recompute the focused widget stack. It should be:

[..., Toolbar, DropDownButton, TooltipDialog, "Set" button]

However it's ending up as:

[..., Toolbar, TooltipDialog, "Set" button]

That causes the DropDownButton to get a blur event, and it closes the drop down, without the "Set" button ever getting a click event.

The DropDownButton isn't in the list of focused widgets because it's disabled, and because of this code in focus.js's onTouchNode() method from [22011]:

if(widget && !(by == "mouse" && widget.get("disabled"))){
	newStack.unshift(id);
}

That seems like a bug in focus manager to ignore the button because it's disabled. Probably disabled widgets should be ignored only there's nothing yet in the stack. So, that's the second bug.

comment:6 Changed 7 years ago by bill

FYI, there's already a

this.editor.focus();

call in modTable(), to address this problem. Looks like it just happens too late? Should be before the call to getTableInfo().

Also note that I'm seeing the same problem on IE8 as you describe in IE9 with the "add row before" button in the toolbar.

And finally, note that the "modify table" button in the toolbar displays the same problem I described above about all buttons getting disabled.

comment:7 Changed 7 years ago by bill

Cc: Ed Chatelain added
Milestone: tbd1.9
Owner: changed from Adam Peller to bill
Status: newassigned

I'll check in a fix on trunk. Ed, if it looks OK you or I can backport it.

comment:8 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30506]:

Fix various problems on IE because the toolbar buttons become disabled when the editor's iframe is blurred.

In the same way that dijit/Editor saves and restores selection when the iframe is focused/blurred, we need to save/restore the info on the selected table cell.

Also, modifying the cell color picker code to use the standard way to close a TooltipDialog, via a <button type=submit>.

Refs #9856, fixes #16286 !strict.

comment:9 Changed 7 years ago by bill

In [30507]:

Rollback spurious spacing changes from my IDE, refs #9856, #16286 !strict.

comment:10 Changed 7 years ago by bill

BTW HaraldOnline, your this.editor.focus() approach works pretty well, but the problem is when you have a TooltipDialog open (like the cell color picker), in which case focusing the editor's iframe causes the TooltipDialog to close.

comment:11 Changed 7 years ago by Ed Chatelain

Bill, the fix looks good, but I did find one additional problem with the code setting the cell background color. If you Insert a new table (outside of any existing tables) and try to set a color on a cell you can get a js exception when it walks up parentNodes looking for a color, if it does not find a color before it hits the #document it will get the exception. It is an easy fix to add the check for the document, so I will fix it on this ticket and then I will try to backport all of this.

comment:12 Changed 7 years ago by Ed Chatelain

In [30518]:

fixes #16286 fix one additional problem with the cell color picker, it was getting a js exception when trying to set a color on a cell in a newly added table because it was missing a check for editor.document. !strict

comment:13 Changed 7 years ago by Ed Chatelain

In [30520]:

fixes #16286 Backport [305006] [30507] and [30518] to 1.8 branch. !strict

comment:14 Changed 7 years ago by Ed Chatelain

In [30521]:

fixes #16286 Backport [305006] [30507] and [30518] to 1.7 branch. !strict

comment:15 Changed 7 years ago by Ed Chatelain

Milestone: 1.91.7.5
Note: See TracTickets for help on using tickets.