Firefox-only - Angular runs into problem with a query parameter named 'watch'

[YukonSaint]

In Firefox only, this line of code: $location.path(window.location.pathname).search('watch=online');

Produces this output in the browser:
http://local.xxx.com/entertainment?watch=function%20watch%28%29%20{%0A%20%20%20%20[native%20code]%0A}&watch=online

Notice the two watch params. I think one is somehow picking up a $scope.watch() function. This line is contained within a $scope.watch function itself.

However if we change the query parameter name to anything else, like watch2 or Watch, the output comes as expected.

$location.path(window.location.pathname).search('Watch=online');

http://local.xxx.com/entertainment?Watch=online

Here is the full function:
$scope.$watch('filters', function(newValue, oldValue){
if(!_.isEqual(oldValue, newValue)){
// update url based on selected filters when it's not deeplinking
$location.path(window.location.pathname).search('Watch=online');
}
}, true);

FYI - I also tried just setting the search manually using window.location.search = 'watch=online'. That works but also reloads the page as expected. What happens next is interesting - angular once again sees the url and adds the second watch param - with the same native function property.

[caitp]

thanks for the report, I'm happy to look into this

[YukonSaint]

Thanks. It could just be something weird about our app. I've been searching for some kind of working example online to test it with but haven't found one.

[caitp]

well, I can't reproduce it with a minimal example, I think we need some more details here.

Could you try and put together a minimal reproduction on http://plnkr.co ?

[YukonSaint]

Yeah I will do that when I can get some time. Unfortunately we're under a crazy deadline at the moment. We are on angular 1.2.12 if that makes any difference.

[caitp]

The simple reproduction isn't broken with 1.2.12 anymore, so I think there's something else going on. But if you can reproduce it I'm happy to look

[YukonSaint]

You did try it in Firefox right?

[caitp]

I did --- FF29 and FF33 (Nightly)

[YukonSaint]

Well I found a good example. It's happening on your website (in Firefox): https://docs.angularjs.org/api/ng/service/$location?watch=test

Turns into:
https://docs.angularjs.org/api/ng/service/$location?watch=function%20watch%28%29%20{%0A%20%20%20%20[native%20code]%0A}&watch=test

If I use some name other than 'watch' it's fine: https://docs.angularjs.org/api/ng/service/$location?watch2=test

[caitp]

Okay, that is something I can work with =)

[caitp]

Okay, so what's happening here is we think that we already have key 'watch' in the object, because FF has Object.prototype.watch (which is not really standard, and is deprecated, but it still exists!).

So what we end up doing is saying obj[key] = [obj[key], val], and this includes a function, which gets toString'd(), and yeah, the results are unpleasant.

I'll try and put together a fix, it shouldn't be too bad.

[YukonSaint]

Thanks a ton for the quick replies Caitlin! If you can give me or help me create some kind of monkey-patch that will work with 1.2.12 that would be awesome as well. We still have to support IE 8 for at least a little while longer - so we haven't been able to upgrade to the latest angular yet.

[caitp]

It's not really monkey-patchable because it's a private API, but I THINK you should be able to just replace if (!obj[key]) in parseKeyValue() with if (!obj.hashOwnProperty(key)) that should be good enough. Might want to make it a bit more strict, like also testing that the value is truthy + an array or string.

And, for IE8, you'd also need to make sure that you can actually use hasOwnProperty for those objects.

Since this is a bug, once I flesh this out a bit more, we can push it to the 1.2 branch anyways.

[YukonSaint]

Thanks a lot caitp. So what is the next step? How do I know when this has been incorporated into a 1.2 hotfix?

[caitp]

This was merged last week, @mattsavino --- I am not sure if we're releasing 1.2 this/next week, but it's a pretty easy patch to apply manually in the mean time.

In any case this will be available soon