Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to 3 dojox module #69

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@IamFake
Copy link
Contributor

commented Feb 24, 2014

dojox.widget.ColorPicker: replace kernel.moduleUrl to require.toUrl
dojox.embed.Flash: change monolithic dojo object to amd modules, and remove hardcoded dojox name
dojox.form.Uploader: remove useless hardcoded dojo name

hardcoded names do not allow you to use the packageMap

ColorPicker: replace kernel.moduleUrl to require.toUrl
Flash: change monolithic dojo object to amd modules, and remove
hardcoded dojox name
Uploader: remove useless hardcoded dojo name

hardcoded names do not allow you to use the packageMap
@@ -37,7 +46,7 @@ define(["dojo"], function(dojo) {
return kwArgs;
}

if(dojo.isIE){
if(has('ie')) {

This comment has been minimized.

Copy link
@dylans

dylans Feb 24, 2014

Member

Should probably test this in IE11, since has('ie') is no longer true for IE11

This comment has been minimized.

Copy link
@IamFake

IamFake Feb 25, 2014

Author Contributor

i think this check should be inside dojo/has, by adding checks against Trident version... as soon as i clone my virtual machine and update to IE11 - i will test it.

This comment has been minimized.

Copy link
@IamFake

IamFake Feb 25, 2014

Author Contributor

oh, i see, dojo have trident checks... but trident and ie checks are independent, i think it is not right...

anyway i've tested embed.Flash in IE11 and it works fine - loading player, playing video and removing embed tag.

This comment has been minimized.

Copy link
@dylans

dylans Feb 25, 2014

Member

The reason the checks are different is that Microsoft requests it, as they feel IE11 is more like other browsers than IE, as they've removed nearly all of the legacy features that IE used to have. So my question was really, for IE11, should it follow the has('ie') path, or the non-IE path?

This comment has been minimized.

Copy link
@IamFake

IamFake Feb 25, 2014

Author Contributor

In that case, i assume we should let him pass non-IE path - as wanted Microsoft and since non-IE path is working (embed instead of object is working good and read flash version through navigator.plugins too). ie-unload code its some sort of clearing, but Flash.destroy do it fine and seems without memory leaks...

@dylans

This comment has been minimized.

Copy link
Member

commented Feb 24, 2014

Thanks for the updates. Do you have a CLA on file?

@dylans

This comment has been minimized.

Copy link
Member

commented Feb 24, 2014

@IamFake

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2014

CLA... "AS-IS" will be ok? :) I'm just new to open source community. ^)

@dylans

This comment has been minimized.

Copy link
Member

commented Feb 25, 2014

@IamFake if you plan to get involved with Dojo, I would recommend the following:

  1. create an account at http://my.dojofoundation.org/ (this will allow you to create bugs, etc.)
  2. Fill out the CLA form at http://dojofoundation.org/about/claForm (it's an online form, it says that you authored the code, and you won't sue Dojo users, etc.)
@IamFake

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2014

I'm filled the CLA form using my email and got "Your Contributor License Agreement has been received and you are now eligible to contribute to all Dojo Foundation projects. Thank you for your contribution!". But there is no instructions how to use it :)

At this time i can't restore my password to my.dojofoundation.org (said "Could not connect to SMTP host").

@dylans

This comment has been minimized.

Copy link
Member

commented Feb 25, 2014

Ok, we've received a CLA from Vitaliy Trushkov, which I assume is you. We will plan to merge these fixes for 1.10 and backport as appropriate to 1.9.x etc. as soon as possible.

@IamFake

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2014

Yes, it's me, thanks

@wkeese

This comment has been minimized.

Copy link
Member

commented Feb 26, 2014

FWIW, I looked over the pull request and it seems fine. I didn't actually try it though.

@dylans

This comment has been minimized.

Copy link
Member

commented Feb 27, 2014

Committed in 4ca4694

Thanks for the contribution!

@dylans dylans closed this Feb 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.