Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(browserTrigger): declare msie variable #9481

Closed
wants to merge 1 commit into from
Closed

fix(browserTrigger): declare msie variable #9481

wants to merge 1 commit into from

Conversation

ksheedlo
Copy link
Contributor

@ksheedlo ksheedlo commented Oct 8, 2014

The variable msie was used in strict mode without declaring it first,
causing both jshint and the module tests to fail. This change fixes the
problem.

@realityking
Copy link
Contributor

It might as well be removed, as far as I can tell it's never used. (See also #9478)

@ksheedlo
Copy link
Contributor Author

ksheedlo commented Oct 8, 2014

True story. I can go and +1 your PR in a sec, but really need Angular's tests to be passing right now as I'm investigating a possible Chrome 38 bug. We might as well delete msie here.

@ksheedlo ksheedlo force-pushed the fix-msie-reference-error branch from 6d22115 to ad1b6a0 Compare October 8, 2014 00:51
@realityking
Copy link
Contributor

Fine with me.

fix(browserTrigger): remove unused msie variable
2e14189 
The variable `msie` was used in strict mode without declaring it first,
causing both jshint and the module tests to fail. This change removes
it, since it was unused.
@ksheedlo ksheedlo force-pushed the fix-msie-reference-error branch from ad1b6a0 to 2e14189 Compare October 8, 2014 01:12
@jeffbcross jeffbcross force-pushed the master branch 2 times, most recently from abdaab7 to 30996f8 Compare October 8, 2014 19:47
@petebacondarwin
Copy link
Contributor

But this is covered by the .jshintrc file for this module no?

https://github.com/angular/angular.js/blob/master/src/ngScenario/.jshintrc#L8

@tbosch tbosch self-assigned this Oct 9, 2014
@tbosch
Copy link
Contributor

tbosch commented Oct 9, 2014

Yes, msie is not used in this file.
Closing this issue in favor of #9478 that also removes it.

@tbosch tbosch closed this Oct 9, 2014
@tbosch
Copy link
Contributor

tbosch commented Oct 9, 2014

@petebacondarwin: the global msie is used inside of src/ngScenario/dsl.js, and it works as we bundle angular with scenario runner in the same function closure.

@petebacondarwin
Copy link
Contributor

@tbosch - Yes, but I didn't understand why it would be failing for @ksheedlo since we do declare it...

@realityking
Copy link
Contributor

The original problem was a missing var in front of the declaration, that caused the failure as the function is in strict mode an uninitialised variables are not allowed. (Also causes a runtime error)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@tbosch tbosch

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ksheedlo @realityking @petebacondarwin @tbosch @mary-poppins