Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

fix(translateCloak): incorrect element reference, inappropriate decloak at onReady, inappropriate decloak at $translateChangeSuccess #1694

Closed

Conversation

mtwar
Copy link

@mtwar mtwar commented Feb 8, 2017

In some instances, the element actually injected and populated in the DOM appears to be a copy of the template element passed to $compile. In this situation, although translate-cloak's logic operates normally, the changes apply to the template, not the actual DOM content, so the cloak is never removed. This fix changes the operative element reference post-compile to the element reference provided to the postLink function, which always references the actual DOM content, thereby propagating the changes as expected. The erroneous behavior was observed when applying translate-cloak inside a directive template

The cloaked element always decloaks at onReady, even if a translationId is provided to trigger the cloak/decloak process. This behavior is inappropriate, as can be observed when using $translatePartialLoader; adding a new part results in additional strings being loaded, but the cloak does not wait for the given translationId to resolve, resulting in a flash of untranslated content. The solution is to only register the onReady handler if an explicit translationId to listen for has not been provided.

After fixing the timing and lifecycle issues in $translate.refresh, I realized that the $translateChangeSuccess handler is actually a bug; it was needed before to trigger the decloak because the $translate.then call in the $observe('translateCloak') handler would inappropriately experience a cache miss if it was invoked during a $translate.refresh cycle, but now that I fixed that I see that the $translateChangeSuccess decloak is inappropriately decloaking elements before they are ready to be decloaked, causing a FOUC. It makes sense that this handler would not be necessary, as the expected behavior when specifying a decloak key is for the element to decloak if and only if the specified translation is loaded successfully. This change is dependent on first merging my fixes to $translate.refresh or there will be regressions where the cloak is never removed.

…ak at onReady, inappropriate decloak at $translateChangeSuccess

In some instances, the element actually injected and populated in the DOM appears to be a copy of the template element passed to $compile. In this situation, although translate-cloak's logic operates normally, the changes apply to the template, not the actual DOM content, so the cloak is never removed. This fix changes the operative element reference post-compile to the element reference provided to the postLink function, which always references the actual DOM content, thereby propagating the changes as expected. The erroneous behavior was observed when applying translate-cloak inside a directive template

The cloaked element always decloaks at onReady, even if a translationId is provided to trigger the cloak/decloak process. This behavior is inappropriate, as can be observed when using $translatePartialLoader; adding a new part results in additional strings being loaded, but the cloak does not wait for the given translationId to resolve, resulting in a flash of untranslated content. The solution is to only register the onReady handler if an explicit translationId to listen for has not been provided.

After fixing the timing and lifecycle issues in $translate.refresh, I realized that the $translateChangeSuccess handler is actually a bug; it was needed before to trigger the decloak because the $translate.then call in the $observe('translateCloak') handler would inappropriately experience a cache miss if it was invoked during a $translate.refresh cycle, but now that I fixed that the $translateChangeSuccess decloak is inappropriately decloaking elements before they are ready to be decloaked. It makes sense that this handler would not be necessary, as the expected behavior when specifying a decloak key is for the element to decloak if and only if the specified translation is loaded successfully. This change is dependent on first merging my fixes to $translate.refresh or there will be regressions in the cloak.
@mtwar
Copy link
Author

mtwar commented Feb 8, 2017

This is not safe to merge unless #1693 is also merged.

@knalli
Copy link
Member

knalli commented Feb 11, 2017

This fix changes the operative element reference post-compile to the element reference provided to the postLink function, which always references the actual DOM content, thereby propagating the changes as expected. The erroneous behavior was observed when applying translate-cloak inside a directive template

Uh, .. good point. Gotcha.

@knalli knalli added this to the 2.14.0 milestone Feb 11, 2017
knalli pushed a commit that referenced this pull request Feb 11, 2017
…ak at onReady, inappropriate decloak at $translateChangeSuccess

In some instances, the element actually injected and populated in the DOM appears to be a copy of the template element passed to $compile. In this situation, although translate-cloak's logic operates normally, the changes apply to the template, not the actual DOM content, so the cloak is never removed. This fix changes the operative element reference post-compile to the element reference provided to the postLink function, which always references the actual DOM content, thereby propagating the changes as expected. The erroneous behavior was observed when applying translate-cloak inside a directive template

The cloaked element always decloaks at onReady, even if a translationId is provided to trigger the cloak/decloak process. This behavior is inappropriate, as can be observed when using $translatePartialLoader; adding a new part results in additional strings being loaded, but the cloak does not wait for the given translationId to resolve, resulting in a flash of untranslated content. The solution is to only register the onReady handler if an explicit translationId to listen for has not been provided.

After fixing the timing and lifecycle issues in $translate.refresh, I realized that the $translateChangeSuccess handler is actually a bug; it was needed before to trigger the decloak because the $translate.then call in the $observe('translateCloak') handler would inappropriately experience a cache miss if it was invoked during a $translate.refresh cycle, but now that I fixed that the $translateChangeSuccess decloak is inappropriately decloaking elements before they are ready to be decloaked. It makes sense that this handler would not be necessary, as the expected behavior when specifying a decloak key is for the element to decloak if and only if the specified translation is loaded successfully. This change is dependent on first merging my fixes to $translate.refresh or there will be regressions in the cloak.

PR #1694
Solves #1556
Solves #1480
@knalli
Copy link
Member

knalli commented Feb 11, 2017

Strike! Just another long outstanding issue fixed. Landed as a4d2795

@mtwar Thank you!

Ehm..: :)

@knalli knalli closed this Feb 11, 2017
knalli added a commit that referenced this pull request Mar 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants