Opened 10 years ago

Closed 10 years ago

Last modified 9 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 10 years ago.
Proposed change to FileUploader?
12273-FileInputAuto.diff (538 bytes) - added by Kenneth G. Franqueiro 10 years ago.
Similar minimal change for FileInputAuto?
12273-leftovers.diff (1.0 KB) - added by Kenneth G. Franqueiro 10 years ago.

Download all attachments as: .zip

Change History (28)

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

Description: modified (diff)

comment:2 Changed 10 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 10 years ago by Kris Zyp

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

comment:4 Changed 10 years ago by Eugene Lazutkin

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

comment:5 Changed 10 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 10 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 10 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 10 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 10 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 10 years ago by bill

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

comment:11 Changed 10 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 10 years ago by Kenneth G. Franqueiro

Attachment: 12273-FileUploader.diff added

Proposed change to FileUploader?

Changed 10 years ago by Kenneth G. Franqueiro

Attachment: 12273-FileInputAuto.diff added

Similar minimal change for FileInputAuto?

comment:12 Changed 10 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 10 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 10 years ago by Eugene Lazutkin

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

comment:15 Changed 10 years ago by Mike Wilcox

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

comment:16 Changed 10 years ago by Mike Wilcox

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

comment:17 Changed 10 years ago by Adam Peller

Owner: changed from Mike Wilcox to dante

On to phiggins...

comment:18 Changed 10 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 10 years ago by Mike Wilcox

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

comment:20 Changed 10 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 10 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 10 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 10 years ago by Kenneth G. Franqueiro

Attachment: 12273-leftovers.diff added

comment:23 Changed 10 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 10 years ago by Adam Peller

Milestone: future1.6

comment:25 Changed 9 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.