mike-jumper commented on a change in pull request #453: GUACAMOLE-903: Improved
Chinese internationalization support
URL: https://github.com/apache/guacamole-client/pull/453#discussion_r362706021
##########
File path:
guacamole/src/main/webapp/app/form/controllers/languageFieldController.js
##########
@@ -40,7 +40,7 @@ angular.module('form').controller('languageFieldController',
['$scope', '$inject
// Retrieve defined languages
languageService.getLanguages().then(function languagesRetrieved(languages)
{
- $scope.$apply(function updateLanguageOptions() {
+ $scope.$applyAsync(function updateLanguageOptions() {
Review comment:
> I have explained the reason for this change in JIRA.
Not yet - the JIRA issue currently says "fixed ... replace "$apply" to
"$applyAsync"" and links to this commit. Please clarify on the issue, when you
can, the actual underlying nature of the issue being fixed.
> I found that there is a script error in the front end when setting the
default internationalization. This error is caused by the wrong use of angular
ajax request.
`$apply()` and `$applyAsync()` do not deal with AJAX, nor is AJAX involved
with the internationalization system. The error in question (infinite digest)
results from `$apply()` being used within a digest cycle.
I see the same errors you do, but please describe on the issue the nature of
the issue being fixed (why current usage of `$apply()` leads to an infinite
digest error). The reason that `$apply()` is incorrect here (ie: what
circumstances would cause this particular call to be invoked within an active
digest) needs to be known.
Splitting this specific fix into its own pull request (rather than
piggybacking it on GUACAMOLE-903) should also be done.
> Unfortunately I don't have the ability to write unit tests for this
modification at this time because it is beyond my ability to do so.
I'm not sure what unit test would need to be written for this, but don't
worry - there's no need in this case. All that is needed is (1) to split this
change such that it only covers the scope of the specific issue and (2) to
document the nature of the issue in JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
