BREAKING CHANGE:
This commit introduces the notion of "private" properties (properties
whose names begin and/or end with an underscore) on the scope chain.
These properties will not be available to Angular expressions (i.e. {{
}} interpolation in templates and strings passed to `$parse`) They are
freely available to JavaScript code (as before).
Motivation
----------
Angular expressions execute in a limited context. They do not have
direct access to the global scope, Window, Document or the Function
constructor. However, they have direct access to names/properties on
the scope chain. It has been a long standing best practice to keep
sensitive APIs outside of the scope chain (in a closure or your
controller.) That's easier said that done for two reasons: (1)
JavaScript does not have a notion of private properties so if you need
someone on the scope chain for JavaScript use, you also expose it to
Angular expressions, and (2) the new "controller as" syntax that's now
in increased usage exposes the entire controller on the scope chain
greatly increaing the exposed surface. Though Angular expressions are
written and controlled by the developer, they (1) typically deal with
user input and (2) don't get the kind of test coverage that JavaScript
code would. This commit provides a way, via a naming convention, to
allow publishing/restricting properties from controllers/scopes to
Angular expressions enabling one to only expose those properties that
are actually needed by the expressions.
When using ngIf with ngInclude on the same element, ngIf previously did not remove
elements added by ngInclude. Similarly, when using ngIfStart/End, ngIf will miss
elements added between the start/end markers added after ngIf is linked.
This commit changes the behavior of ngIf to add a comment node at the end of its
elements such that elements between the starting comment and this ending comment
are removed when ngIf's predicate does not hold.
Recently we changed the priority of attribute interpolation directive to -100
to ensure that it executes early in the post linking phase. This causes issues
with when terminal directives are placed on elements with attribute bindings
because the terminal directive will usually have 0 or higher priority which
results in attr interpolation directive not being applied to the element.
To fix this issue I'm switching the priority back to 100 and making moving the
binding setup into the pre-linking function.
This means that:
- terminal directives with priority lower than 100 will not affect the attribute
binding
- if a directive wants to add or alter bindings it can do so in the pre-linking
phase, as long as the priority of this directive is more than 100
- all post-linking functions will execute after the attribute binding has been
set up
- all pre-linking functions with directive priority lower than 100 will execute
after the attribute bindings have been setup
BREAKING CHANGE: the attribute interpolation (binding) executes as a directive
with priority 100 and the binding is set up in the pre-linking phase. It used
to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding
was setup in the post-linking phase.
Closes#4525Closes#4528Closes#4649
Skip addClass animations if the element already contains the class that is being
added to element. Also skip removeClass animations if the element does not contain
the class that is being removed.
Closes#4401Closes#2332
When we re-enter compilation either due to async directive templates or element transclude directive
we need to keep track of controllers to instantiate during linking.
This piece of info was missing when re-entering compilation and that's what this commit fixes.
I also reordered the properties in the previousCompileContext object.
Closes#4434Closes#4616
When we refactored , we broke the csp mode because the previous implementation
relied on the fact that it was ok to lazy initialize the .csp property, this
is not the case any more.
Besides, we need to know about csp mode during bootstrap and avoid injecting the
stylesheet when csp is active, so I refactored the code to fix both issues.
PR #4411 will follow up on this commit and add more improvements.
Closes#917Closes#2963Closes#4394Closes#4444
BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not
supported any more. Please use data-ng-csp instead.
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.
The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.
This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See #4357.
Closes#3893Closes#4217Closes#3307
The routeUtils.js file was declaring a number of functions that were
leaking into other modules such as ngMocks causing tests to pass
incorrectly.
Closes#4360
The location service, and other portions of the application,
were relying on a complicated regular expression to get parts of a URL.
But there is already a private urlUtils provider,
which relies on HTMLAnchorElement to provide this information,
and is suitable for most cases.
In order to make urlUtils more accessible in the absence of DI,
its methods were converted to standalone functions available globally.
The urlUtils.resolve method was renamed urlResolve,
and was refactored to only take 1 argument, url,
and not the 2nd "parse" boolean.
The method now always returns a parsed url.
All places in code which previously wanted a string instead of a parsed
url can now get the value from the href property of the returned object.
Tests were also added to ensure IPv6 addresses were handled correctly.
Closes#3533Closes#2950Closes#3249
Issue: multi-elements ng-repeat (ng-repeat-start, ng-repeat-end) can contain elements with a trancluding directive. This directive changes content of the row (template) and ng-repeat does not work correctly (when removing/moving rows), because ng-repeat works with the original template (elements).
This changes ng-repeat behavior to traverse the DOM to find current elements everytime we are moving/removing rows (if the template has multiple elements).
Closes#3104
Since c785267e jqLite uses setAttribute (rather than className property) in order to change classes. Some elements (eg. Comment) do not have this method which blows up.
jQuery silently ignores these method calls (because it uses className), so to get the same behavior as jQuery, we check for setAttribute method first.
This reverts commit 3a65822023.
The change cased regressions in third party components that require
promises from getter functions not to be unwrapped.
Since we have deprecated the promise unwrapping support in $parse it
doesn't make much sense to fix this issue and deal with regressions in
third party code.
Closes#4158
This commit disables promise unwrapping and adds
$parseProvider.unwrapPromises() getter/setter api that allows developers
to turn the feature back on if needed. Promise unwrapping support will
be removed from Angular in the future and this setting only allows for
enabling it during transitional period.
If the unwrapping is enabled, Angular will log a warning about each
expression that unwraps a promise (to reduce the noise, each expression
is logged only onces). To disable this logging use
`$parseProvider.logPromiseWarnings(false)`.
Previously promises found anywhere in the expression during expression
evaluation would evaluate to undefined while unresolved and to the
fulfillment value if fulfilled.
This is a feature that didn't prove to be wildly useful or popular,
primarily because of the dichotomy between data access in templates
(accessed as raw values) and controller code (accessed as promises).
In most code we ended up resolving promises manually in controllers
or automatically via routing and unifying the model access in this way.
Other downsides of automatic promise unwrapping:
- when building components it's often desirable to receive the
raw promises
- adds complexity and slows down expression evaluation
- makes expression code pre-generation unattractive due to the
amount of code that needs to be generated
- makes IDE auto-completion and tool support hard
- adds too much magic
BREAKING CHANGE: $parse and templates in general will no longer
automatically unwrap promises. This feature has been deprecated and
if absolutely needed, it can be reenabled during transitional period
via `$parseProvider.unwrapPromises(true)` api.
Closes#4158Closes#4270
The trigger handler event in jqLite takes an event object as a second
parameter, but jQuery requires an array of parameters. This is causing
the touchend event to not come thtough in the click handler when jQuery
is loaded.
Existing documentation implies that an Event object should be available
as `$event` on swipe directives, which previously was only working for
`ng-click`.
Closes#4071Closes#4321
`checkboxInputType` and `ngList` directives need to have special logic for whether
they are empty or not. Previously this had been hard coded into their
own directives or the `ngRequired` directive. This made it difficult to handle
these special cases.
This change factors out the question of whether an input is empty into a method
`$isEmpty` on the `ngModelController`. The `ngRequired` directive now uses this
method when testing for validity and directives, such as `checkbox` or `ngList`
can override it to apply logic specific to their needs.
Closes#3490, #3658, #2594
The $interval service simplifies creating and testing recurring tasks.
This service does not increment $browser's outstanding request count,
which means that scenario tests and Protractor tests will not timeout
when a site uses a polling function registered by $interval. Provides
a workaround for #2402.
For unit tests, repeated tasks can be controlled using ngMock$interval's
tick(), tickNext(), and tickAll() functions.
Objects received from outside AngularJS may have had their `hasOwnProperty`
method overridden with something else. In cases where we can do this without
incurring a performance penalty we call directly on Object.prototype.hasOwnProperty
to ensure that we use the correct method.
Also, we have some internal hash objects, where the keys for the map are provided
from outside AngularJS. In such cases we either prevent `hasOwnProperty` from
being used as a key or provide some other way of preventing our objects from
having their `hasOwnProperty` overridden.
BREAKING CHANGE: Inputs with name equal to "hasOwnProperty" are not allowed inside
form or ngForm directives.
Before, inputs whose name was "hasOwnProperty" were quietly ignored and not added
to the scope. Now a badname exception is thrown.
Using "hasOwnProperty" for an input name would be very unusual and bad practice.
Either do not include such an input in a `form` or `ngForm` directive or change
the name of the input.
Closes#3331
Prevent the obj.$delete instance method from sending the resource as the request body. This commit uses the existing hasBody boolean to only set httpConfig.data for methods which should have a request body.
Closes#4280
This reduces memory consumption of parsed angular expressions and
speeds up parsing.
This JSPerf case demonstrates the performance boost:
http://jsperf.com/closure-vs-prototype-ngparser
Chrome: 1.5–2x boost
FF: slightly slower (I would love to know why)
IE: 4x boost
To be clear, this doesn't have any impact on runtime performance
of expressions as demostrated in this JSPerf:
http://jsperf.com/angular-parser-changesCloses#3681
previously the compile/link fns executed in this order controlled via priority:
- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityHigh, PostLinkPriorityMedium, PostLinkPriorityLow
This was changed to:
- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityLow, PostLinkPriorityMedium , PostLinkPriorityHigh
Using this order the child transclusion directive that gets replaced
onto the current element get executed correctly (see issue #3558),
and more generally, the order of execution of post linking function
makes more sense. The incorrect order was an oversight that has
gone unnoticed for many suns and moons.
(FYI: postLink functions are the default linking functions)
BREAKING CHANGE: the order of postLink fn is now mirror opposite of
the order in which corresponding preLinking and compile functions
execute.
Very few directives in practice rely on order of postLinking function
(unlike on the order of compile functions), so in the rare case
of this change affecting an existing directive, it might be necessary
to convert it to a preLinking function or give it negative priority
(look at the diff of this commit to see how an internal attribute
interpolation directive was adjusted).
Closes#3558
Previously we would stop the compilation for both regular and element
transclusion directives which was wrong. Only element transclusion directives
should be terminal.
HTML to be sanitized that contains a DOCTYPE declaration were causing
the HTML parser to throw an error. Now the parser correctly removes
the declarations when sanitizing HTML.
Closes#3931
Refactored `replacedUrl` to store the new URL on both
`location.replace` and setting `location.href` directly to handle
delays in the actual location value change in IE.
Closes#2802
Fix wrong behaviour that didn't allow 'data-on' and 'on' element attributes
to be interpolated by $compile. The regex now accepts any string beginning
with 'on' and with at least one more English letter.
This is a breaking change. To migrate to the new behavior,
delete or set headers to `undefined` to avoid having them sent.
To restore the old behavior, override `$httpBackendProvider`
with the old implementation.
Closes#2984
The `XMLHttpRequest.send` spec defines different semantics for `null`
than for an empty String: an empty String should be sent with a
`Content-Type` of `text/plain`, whereas `null` should have no
`Content-Type` header set.
Closes#2149
This feature adds similar functionality to what `$ControllerProvider.register`
and `$CompileProvider.directive` currently provide by allowing a map of filter
name/factories to be passed as the sole argument to `$FilterProvider.register`
to register all of the specified filters.
Closes#4036Closes#4091
How did compiling a templateUrl (async) directive with `replace:true` work before this commit?
1/ apply all directives with higher priority than the templateUrl directive
2/ partially apply the templateUrl directive (create `beforeTemplateNodeLinkFn`)
3/ fetch the template
4/ apply second part of the templateUrl directive on the fetched template
(`afterTemplateNodeLinkFn`)
That is, the templateUrl directive is basically split into two parts (two `nodeLinkFn` functions),
which has to be both applied.
Normally we compose linking functions (`nodeLinkFn`) using continuation - calling the linking
function of a parent element, passing the linking function of the child elements as an argument. The
parent linking function then does:
1/ execute its pre-link functions
2/ call the child elements linking function (traverse)
3/ execute its post-link functions
Now, we have two linking functions for the same DOM element level (because the templateUrl directive
has been split).
There has been multiple issues because of the order of these two linking functions (creating
controller before setting up scope locals, running linking functions before instantiating
controller, etc.). It is easy to fix one use case, but it breaks some other use case. It is hard to
decide what is the "correct" order of these two linking functions as they are essentially on the
same level.
Running them side-by-side screws up pre/post linking functions for the high priority directives
(those executed before the templateUrl directive). It runs post-linking functions before traversing:
```js
beforeTemplateNodeLinkFn(null); // do not travers
afterTemplateNodeLinkFn(afterTemplateChildLinkFn);
```
Composing them (in any order) screws up the order of post-linking functions. We could fix this by
having post-linking functions to execute in reverse order (from the lowest priority to the highest)
which might actually make a sense.
**My solution is to remove this splitting.** This commit removes the `beforeTemplateNodeLinkFn`. The
first run (before we have the template) only schedules fetching the template. The rest (creating
scope locals, instantiating a controller, linking functions, etc) is done when processing the
directive again (in the context of the already fetched template; this is the cloned
`derivedSyncDirective`).
We still need to pass-through the linking functions of the higher priority directives (those
executed before the templateUrl directive), that's why I added `preLinkFns` and `postLinkFns`
arguments to `applyDirectivesToNode`.
This also changes the "$compile transclude should make the result of a transclusion available to the
parent directive in post- linking phase (templateUrl)" unit test. It was testing that a parent
directive can see the content of transclusion in its pre-link function. That is IMHO wrong (as the
`ngTransclude` directive inserts the translusion in its linking function). This test was only passing because of
c173ca4128, which changed the behavior of the compiler to traverse
before executing the parent linking function. That was wrong and also caused the #3792 issue, which
this change fixes.
Closes#3792Closes#3923Closes#3935Closes#3927
jqLite previously used `elt.className` to add and remove classes from a DOM Node, but
because the className property is not writable on SVG elements, it doesn't work with
them. This patch replaces accesses to `className` with `get/setAttribute`.
`classList` was also considered as a solution, but because only IE10+ supports it, we
have to wait. :'(
The JqLiteAddClass/JQLiteRemoveClass methods are now also used directly by $animate
to work around the jQuery not being able to handle class modifications on SVG elements.
Closes#3858
The problem was in keeping the values of `attrNameStart` and `attrNameEnd` between directive loop iterations which lead to the compiler looking for multi-element ranges for any directives that happened to be in the directive list after one that was applied on a range. For instance, having a ng-repeat-start and ng-class on a single element with ng-repeat being resolved first made the compiler look for an ng-repeat-end for both ng-repeat and ng-class because the `attrNameEnd` was not reset to a falsy value before the second iteration. As the result, an exception saying the block end element could not be found and the second directive was not actually applied.
Closes#4002
Previously if the collection model was set to undefined on the first digest,
the repeater would get confused and not use the correct tracking function
for associating model with dom elements in the repeater.
Closes#4145Closes#3964
Ref: https://github.com/angular/angular.js/pull/4045
I have this sinking feeling that support this use case sort of
encourages binding to function that blindly trust some html. For now,
I'm fixing the issue while I think about the use cases some more.
In the case of a function that performs any non-trivial work before
wrapping the value (e.g. the showdown filter in issue #3980, or the
binding to a simply wrapper function in issue #3932 if it did anything
meaty), this fix makes it "work" - but performance is going to suck -
you should bind to some other thing on scope that watches the actual
source and adjusts itself when that changes (e.g. the showdown filter.)
For the case of the wrapper in #3932, if one isn't performing
sanitization or some such thing - then you the developer has insight
into why that value is safe in that particular context - and it should
be available simply by name and not as a result of a function taking any
arbitrary input to make auditing of security a little saner.
Closes#3932, #3980
BREAKING CHANGE: ngInclude's priority is now set to 1000
It's quite rare for anyone to depend on explicity directive priority,
but if a custom directive that needs to run before ngInclude exists,
it should have its priority checked and adjusted if needed.
Closes#3793
Previously, the check that Application should return a new $window and
$document had the arguments reversed in the first call to navigateTo;
thus, the subsequent check of inequality of $window and $document in the
next navigateTo call would always pass.
This corrects the argument order, which makes this test not succeptible
to false positives.
onAnimationProgress now checks the event's elapsedTime property before
checking the originalEvent.elapsedTime property.
Use browserTrigger with elapsedTime parameter to trigger animation events
BREAKING CHANGE: browserTrigger now uses an eventData object instead of direct parameters for mouse events.
To migrate, place the `keys`,`x` and `y` parameters inside of an object and place that as the third parameter
for the browserTrigger function.
It's great that IE11 wants to be compatible enough that it doesn't want
to be special cased and treated differently.
However, as long as one has to have a different code path for IE than
for the other supported browsers, we still need to detect and special
case it. For instance, our URL parsing code still needs the same
workaround the we used for IE10. We still see the same Access denied /
TypeError exceptions when setting certain values. FYI, Angular doesn't
generally blindly test for IE – we also check the version number.
Thanks to modern.ie for the free IE11 test VM.
Closes#3682
This reverts commit 42af8eada2.
This turned out to be a bad idea as it prevents us from moving the
time forward and asserting that the component state didn't change
due to the scheduled task executing too early.
This change causes a new $digest to be scheduled in the next tick if
a task was was sent to the $evalAsync queue from outside of a $digest
or an $apply.
While this mode of operation is not common for most of the user code,
this change means that $q promises that utilze $evalAsync queue to
guarantee asynchronicity of promise apis will now also resolve outside
of a $digest, which turned out to be a big pain point for some developers.
The implementation ensures that we don't do more work than needed and
that we coalese as much work as possible into a single $digest.
The use of $browser instead of setTimeout ensures that we can mock out
and control the scheduling of "auto-flush", which should in theory
allow all of the existing code and tests to work without negative
side-effects.
Closes#3539Closes#2438
When $timeout#flush is called with a delay and no task can be flushed within that
delay, the current time should not be updated as that gets the mock into an inconsistent
state.
BREAKING CHANGE: if a tests was written around the buggy behavior the delays might be off now
This would typically not be a problem, but because of the previous breaking change in
$timeout.flush, the combination of two might be confusing and that's why we are documenting
it.
Old behavior:
```
doSomething(); //schedules task to execute in 500ms from now
doOtherStuff(); //schedules task to execute in 600ms from now
try {
$timeout.flush(300); // throws "no task to be flushed" exception
} catch(e) {};
$time.flush(200); //flushes only doSomething() task
```
New behavior:
```
doSomething(); //schedules task to execute in 500ms from now
doOtherStuff(); //schedules task to execute in 600ms from now
try {
$timeout.flush(300); // throws "no task to be flushed" exception
} catch(e) {};
$time.flush(200); // throws "no task to be flushed" exception again
// because previous exception didn't move the time forward
```
Fixed test:
```
doSomething(); //schedules task to execute in 500ms from now
doOtherStuff(); //schedules task to execute in 600ms from now
try {
$timeout.flush(300); // throws "no task to be flushed" exception
} catch(e) {};
$time.flush(500); // flushes only doSomething() task
```
When calling $timeout.flush with or without a delay an exception should
be thrown if there is nothing to be flushed.
This prevents tests from flushing stuff unnecessarily.
BREAKING CHANGE: calling $timeout.flush(delay) when there is no task to be flushed
within the delay throws an exception now.
Please adjust the delay or remove the flush call from your tests as the exception
is a signed of a programming error.
