Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#12273 closed defect (fixed)

IE9: dojox nonstandard usages of document.createElement

Reported by: Kenneth G. Franqueiro Owned by: dante
Priority: high Milestone: 1.5.2
Component: Dojox Version: 1.6.0b1
Keywords: ie9 Cc: dante, Mike Wilcox, Kris Zyp, Adam Peller
Blocked By: Blocking:

Description (last modified by Kenneth G. Franqueiro)

We've already found and addressed issues in dojo core and dijit involving IE-specific code paths calling document.createElement with an entire HTML fragment, which IE9 doesn't like so much. There are 3 cases of this that I've found in dojox, in the following places:

dojox.form.FileInputAuto (line 107) - can reproduce issue on http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/form/tests/test_FileInput.html by browsing for a file on any of the FileUploadAuto test widgets, then making it lose focus.

dojox.form.FileUploader (line 1093) - can reproduce issue simply by loading page http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/form/tests/test_FileUploader.html?forceNoFlash

dojox.io.windowName (line 129) - can reproduce issue on http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/io/tests/windowName.html by clicking "get data direct" button and entering URL.

Attachments (3)

12273-FileUploader.diff (562 bytes) - added by Kenneth G. Franqueiro 8 years ago.
Proposed change to FileUploader?
12273-FileInputAuto.diff (538 bytes) - added by Kenneth G. Franqueiro 8 years ago.
Similar minimal change for FileInputAuto?
12273-leftovers.diff (1.0 KB) - added by Kenneth G. Franqueiro 8 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 8 years ago by Kenneth G. Franqueiro

Description: modified (diff)

comment:2 Changed 8 years ago by Adam Peller

Cc: dante Mike Wilcox Kris Zyp added

by cc, can each of you take a look and fix in the next few days for 1.6, if it's trivial?

comment:3 Changed 8 years ago by Kris Zyp

(In [23764]) Fix element creation for IE9, refs #12273

comment:4 Changed 8 years ago by Eugene Lazutkin

Cc: Adam Peller added
Milestone: tbdfuture
Owner: changed from Adam Peller to Eugene Lazutkin
Status: newassigned

comment:5 Changed 8 years ago by Eugene Lazutkin

BTW, what is wrong with using dojo.create(), dojo.place(), or dojo._toDom() in all those instances?

comment:6 Changed 8 years ago by bill

I think that's fine, I did it in [23745] and [23746], not sure why Kris didn't do it in [23764] but that might be tricky since it's an <iframe>.

comment:7 Changed 8 years ago by Mike Wilcox

IE9 did not cooperate with FileUploader?. I used dojo.create but I'm not sure doing that properly set the enctype for the form. When I tried removing the enctype it threw even more errors elsewhere.

comment:8 Changed 8 years ago by bill

dojo.place() might work better, IE is infamous for problems setting attributes after an element is created (ex: the "type" attribute on <input> controls).

comment:9 Changed 8 years ago by dante

fwiw, IE is only "infamous" in that it only allows you to set the type on a node BEFORE it is attached to the dom. iirc, this is to spec.

var n = document.CreateElement("input");
n.type = "checkbox"
document.body.appendChild(n)

Switching lines 2 and 3 will cause exceptions.

I believe the behavior is "in the dom for the first time", so create(), attr(), place(), attr() would break, but create() attr() place() is fine.

A relevant has.js test is: https://github.com/phiggins42/has.js/blob/master/detect/dom.js#L88 (the "dom-create-attr" test, should lines change one day)

comment:10 Changed 8 years ago by bill

Well, it also has problems with create() attr() place(), see #8660 for example.

comment:11 Changed 8 years ago by Kenneth G. Franqueiro

I tried doing a minimal edit using dojo.create for the FileUploader code and it seemed to propagate Content-Type: multipart/form-data properly; is that what the question is about, or is there any other purpose to encType?

I'll attach a patch with my proposed change. Tested successfully in IE6, 8, 9RC (using the HTML instance on test_FileUploaderForm.html.

Changed 8 years ago by Kenneth G. Franqueiro

Attachment: 12273-FileUploader.diff added

Proposed change to FileUploader?

Changed 8 years ago by Kenneth G. Franqueiro

Attachment: 12273-FileInputAuto.diff added

Similar minimal change for FileInputAuto?

comment:12 Changed 8 years ago by Mike Wilcox

@kgf, did you test that? Because it looks like what I tried. You may get past the initial parsing error in IE9, but then when you attempt an upload you start getting errors in io.iframe.

comment:13 Changed 8 years ago by Kenneth G. Franqueiro

Yes, I tested successfully with the test_FileUploaderForm page, as I said in my previous comment. All I did was fill in each of the fields in the right-hand form, pick a file (or multiple files) to upload, then click the button. Everything seemed to go through, no problem. upload.txt reflected all the fields were filled in.

Are you sure your dojo.io.iframe is up to date from trunk? There were IE9 fixes in that too about 2 weeks ago (done by bill).

comment:14 Changed 8 years ago by Eugene Lazutkin

Owner: changed from Eugene Lazutkin to Mike Wilcox
Status: assignednew

comment:15 Changed 8 years ago by Mike Wilcox

(In [23826]) Fixes #12308 Refs #12273 - fixed html ie9 upload bug.

comment:16 Changed 8 years ago by Mike Wilcox

Thanks kgf. I was out of date and that was the problem. Fixed and committed.

comment:17 Changed 8 years ago by Adam Peller

Owner: changed from Mike Wilcox to dante

On to phiggins...

comment:18 Changed 8 years ago by Kenneth G. Franqueiro

Good deal, just tested FileUploader with the change, in all IEs 6-9. Not to split hairs, but I assume it'd be safe for us to remove the commented-out code in that method, then? :)

Meanwhile, would anyone like to review/retest my patch for FileInputAuto so we can close down this ticket?

comment:19 Changed 8 years ago by Mike Wilcox

(In [23828]) Removing commented code from FileUploader? Refs #12273

comment:20 Changed 8 years ago by Noddie

I would assume that the fix in FileInputAuto? could also be done in the same way as FileUploader?, ie make it run the "standard" codepath for IE9 and only use the special for IE8 and below as per today.

if(dojo.isIE && dojo.isIE < 9){ 

comment:21 Changed 8 years ago by Adam Peller

you only need to say if(dojo.isIE < 9) Of course, it's best avoiding the browser detection, if possible.

comment:22 Changed 8 years ago by Kenneth G. Franqueiro

Noddie is right, upon testing, that approach seems to work fine for FileInputAuto as well. I'll attach a patch with that fix for FileInputAuto as well as omitting the unnecessary extra test in FileUploader. Can someone review it?

Changed 8 years ago by Kenneth G. Franqueiro

Attachment: 12273-leftovers.diff added

comment:23 Changed 8 years ago by Kenneth G. Franqueiro

Resolution: fixed
Status: newclosed

(In [23868]) Remaining codepath revisions for IE9 in dojox.form, fixes #12273.

comment:24 Changed 8 years ago by Adam Peller

Milestone: future1.6

comment:25 Changed 8 years ago by Kenneth G. Franqueiro

Milestone: 1.61.5.2

Updating milestone to 1.5.2 to reflect inclusion in changeset [26956] for ticket #14199.

Note: See TracTickets for help on using tickets.