Code review comment for lp:~adiroiban/launchpad/bug-540105

Revision history for this message
Adi Roiban (adiroiban) wrote :

Hi Edwin,

I moved the try/catch inside the domready function call.

I did some testing on Firefox and Chromium on Ubuntu Karmic, and without the try/catch, a failing domready function call will prevent the loading of all other domready functions.

Here is the latest review.
=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
--- lib/lp/translations/templates/pofile-translate.pt 2010-03-18 17:01:11 +0000
+++ lib/lp/translations/templates/pofile-translate.pt 2010-03-22 14:59:42 +0000
@@ -18,31 +18,41 @@
       registerLaunchpadFunction(insertAllExpansionButtons);

       LPS.use('lp.pofile', function(Y) {
- try {
- Y.on('domready', Y.lp.pofile.updateNotificationBox);
- } catch (e) {
- Y.log(e, "error");
- }
- try {
- Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
- } catch (e) {
- Y.log(e, "error");
- }
- try {
- Y.on('domready', Y.lp.pofile.initializeKeyBindings);
- } catch (e) {
- Y.log(e, "error");
- }
- try {
- Y.on('domready', function(e) {
- Y.lp.pofile.setFocus(autofocus_field);
- });
- } catch (e) {
- Y.log(e, "error");
- }
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.updateNotificationBox();
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.setupSuggestionDismissal();
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.initializeKeyBindings();
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.setFocus(autofocus_field);
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
       });

-
     </script>
     </div>
     <div metal:fill-slot="main">

=== modified file 'lib/lp/translations/templates/translationmessage-translate.pt'
--- lib/lp/translations/templates/translationmessage-translate.pt 2010-03-18 17:01:11 +0000
+++ lib/lp/translations/templates/translationmessage-translate.pt 2010-03-22 14:57:08 +0000
@@ -15,23 +15,29 @@
     </style>
     <script type="text/javascript">
       LPS.use('node', 'lp.pofile', function(Y) {
- try {
- Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
- } catch (e) {
- Y.log(e, "error");
- }
- try {
- Y.on('domready', Y.lp.pofile.initializeKeyBindings);
- } catch (e) {
- Y.log(e, "error");
- }
- try {
- Y.on('domready', function(e) {
- Y.lp.pofile.setFocus(autofocus_field);
- });
- } catch (e) {
- Y.log(e, "error");
- }
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.setupSuggestionDismissal();
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.initializeKeyBindings();
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
+
+ Y.on('domready', function(e) {
+ try {
+ Y.lp.pofile.setFocus(autofocus_field);
+ } catch (e) {
+ Y.log(e, "error");
+ }
+ });
       });
     </script>
     </div>

« Back to merge proposal