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

Standardising code - see commit message #2

Merged
merged 13 commits into from
Dec 15, 2011

Conversation

adamsilver
Copy link
Contributor

...of conditionals etc

@adamsilver
Copy link
Contributor Author

I wasn't sure about the IE8- prong for getEventTarget as it was repeating some of the logic for the "degrades in IE8-" prong.

Also, I have made an assumption that it doesn't need feature detection as your intention was for it to be put in a conditional comment.

Also, the way you have written the IE8- prong suggests that e.srcElement could never be a text node. Is that right?

@gibletto
Copy link
Contributor

Would you use @cc comments to fork or load the script via <!--[if lte IE 8]> or whatever? I guess it depends on if you want one script to handle all cases, or conditionally load a second script when using IE8-

@adamsilver
Copy link
Contributor Author

It would be <!--[if lte IE 8]> (conditional comments) that I am referring to and not conditional compilation (http://www.javascriptkit.com/javatutors/conditionalcompile.shtml)

@gibletto
Copy link
Contributor

Any builder would need to generate the both scripts then, but that's much of a muchness right now I guess!

@adamsilver
Copy link
Contributor Author

Yes, David has mentioned this in CLJ posts, but am waiting for his input on this.

@david-mark
Copy link
Member

Right. Conditional comments as you can't target a specific IE (MSHTML) version with conditional compilation. The idea is that it is up to the developer to determine whether they want one or two files.

As for getEventTarget, it doesn't have any forked renditions. I think I posted two renditions, one that can only be used with an attachListener that degrades in IE 8- and the other one is universal. The builder chooses the one that is appropriate based on the specific degradation point.

But I think we should just create getEventTarget along with attachListener. The forked rendition of attachListener would need to create either the first getEventTarget rendition or a new one that just always returns e.srcElement.

And yes, srcElement is always an element reference (never a text node or anything else). That's an MSHTML-ism that predates all of the standard DOM specifications.

So I will go ahead and merge these. I am too busy to go over them at the moment, will review all of the changes in the near future. I assume that any "pull" I merge can be pushed back out if needed, but I doubt that would be necessary at this stage.

Thanks for helping out!

david-mark pushed a commit that referenced this pull request Dec 15, 2011
Standardising code - see commit message
@david-mark david-mark merged commit 455b142 into david-mark-llc:master Dec 15, 2011
david-mark pushed a commit that referenced this pull request May 3, 2012
xhrSend: fixed bug when receiving successful response and no success han...
@adamsilver adamsilver mentioned this pull request Aug 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants