Launchpad itself

Merge lp:~adiroiban/launchpad/bug-540105 into lp:launchpad

  1. bug-540105
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-540105
Merge into: lp:launchpad
Diff against target: 338 lines (+152/-55)
5 files modified
lib/canonical/launchpad/javascript/translations/pofile.js (+54/-40)
lib/lp/app/templates/base-layout-macros.pt (+3/-0)
lib/lp/translations/templates/pofile-translate.pt (+26/-8)
lib/lp/translations/templates/translationmessage-translate.pt (+17/-7)
lib/lp/translations/windmill/tests/test_pofile_translate.py (+52/-0)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-540105
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+21552@code.launchpad.net

Commit message

Move independent functionality in different JS sandboxes and fix
pofile.js updateNotificationBox() to not fail when there are no notification boxes on the page.

Description of the change

= Bug 540105 =
I noticed this behaviour in edge:

When entering text in the input box for a translation, this textbox does no longer get the focus by automatically selecting the checkbox dot on its left.

This leads to translators not noticing it, pressing "Save" and loosing all their translations.

== Proposed fix ==
Move independent functionality in different JS sandboxes and fix
pofile.js updateNotificationBox() to not fail when there are no notification boxes on the page.

== Pre-implementation notes ==
This branch is a hotfix for this problem.

I have opened bug 540216 and working on improving the general test coverage for pofile.js code.

== Implementation details ==
I have also fixes some other lint warnings.

== Tests ==
lp-test -t pofile_new_translation_autoselect

== Demo and Q/A ==
Login as admin and go to:
https://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+translate

In the "New translation" field, enter some text. The radio button before the text field should be now selected.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/javascript/translations/pofile.js

== JSLint notices ==
No handlers could be found for logger "bzr"
jslint: Lint found in '/home/adi/launchpad/lp-branches/bug-540105/lib/canonical/launchpad/javascript/translations/pofile.js':
Line 303 character 18: Be careful when making functions within a loop. Consider putting the function in a closure.
                },

Line 310 character 18: Be careful when making functions within a loop. Consider putting the function in a closure.
                }, '#' + fields[key], 'down:68+shift+alt', Y, original_stem);

Line 316 character 18: Be careful when making functions within a loop. Consider putting the function in a closure.
                }, '#' + fields[key], 'down:48+shift+alt', Y, fields[key]);

jslint: 1 file to lint.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (5.0 KiB)

Hi Adi,

This is a good improvement. I ran into an annoying problem running the tests.
The tests use the combined javascript file, but "make jsbuild" was not getting
run automatically. I also noticed that translationmessage-translate.pt and
pofile-translate.pt load pofile.js directly, when the <script> tag really
should be added to lib/lp/app/templates/base-layout-macros.pt next to all
the other js files in the ${lp_js} directory.

Since I was confused by some of your changes that I don't think were
necessary, I'm marking this
    needs-fixing

-Edwin

>=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
>--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-02-25 12:56:45 +0000
>+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-17 17:36:08 +0000
>@@ -55,10 +55,15 @@
> node.setStyle('display', 'none');
> });
> hide_anim.run();
>-}
>+};
>
> self.updateNotificationBox = function(e) {
> var notice = Y.one('.important-notice-container');
>+ if (!notice) {

Since Y.one() is expected to return null when the element is not found,
the preferred if-statement is:
    if (notice !== null)

>+ // We have no notice container on this page, this is why there is
>+ // nothing more to be done by this function.
>+ return;
>+ }
> var balloon = notice.one('.important-notice-balloon');
> var dismiss_notice_cookie = ('translation-docs-for-' +
> documentation_cookie);
>@@ -74,6 +79,10 @@
> '.important-notice-cancel-button');
> // Cancel button starts out hidden. If user has JavaScript,
> // then we want to show it.
>+ if (!cancel_button) {

Same here.

>+ // No cancel button was found to attach the action.
>+ return;
>+ }
> cancel_button.setStyle('visibility', 'visible');
> cancel_button.on('click', function(e) {
> e.halt();
>@@ -202,30 +211,34 @@
> }
> // Shift+Alt+n - Go to next page in batch
> if (e.shiftKey && e.altKey && e.keyCode == 78) {
>- if (link = Y.one('#batchnav_next')){
>- window.location.assign(link.get('href'))
>+ link = Y.one('#batchnav_next');
>+ if (link){

Same for these if-statements.

>+ window.location.assign(link.get('href'));
> }
> }
> // Shift+Alt+p - Go to previous page in batch
> if (e.shiftKey && e.altKey && e.keyCode == 80) {
>- if (link = Y.one('#batchnav_previous')){
>- window.location.assign(link.get('href'))
>+ link = Y.one('#batchnav_previous');
>+ if (link){
>+ window.location.assign(link.get('href'));
> }
> }
> // Shift+Alt+a - Go to first page in batch
> if (e.shiftKey && e.altKey && e.keyCode == 65) {
>- if (link = Y.one('#batchnav_first')){
>- window.location.assign(link.get('href'))
>+ link = Y.one('#batchnav_first');
>+ if (link){
>+ window.location.assign(link.get('href'));
> }
> }
> // Shift+Alt+l - Go to last page in batch
> if (e.shiftKey && e.altKey && e.keyCode == 76) {
>- if (link = Y.one('#batchnav_last')){
>- ...

Read more...

review: Needs Fixing (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :
Download full text (8.7 KiB)

Hi,
> Hi Adi,
>
> This is a good improvement. I ran into an annoying problem running the tests.
> The tests use the combined javascript file, but "make jsbuild" was not getting
> run automatically. I also noticed that translationmessage-translate.pt and
> pofile-translate.pt load pofile.js directly, when the <script> tag really
> should be added to lib/lp/app/templates/base-layout-macros.pt next to all
> the other js files in the ${lp_js} directory.

I have moved the pofile.js inclusing in base-layout-macros.pt.

> Since I was confused by some of your changes that I don't think were
> necessary, I'm marking this
> needs-fixing

Thanks for the review

[snip]

> > var notice = Y.one('.important-notice-container');
> >+ if (!notice) {
>
>
> Since Y.one() is expected to return null when the element is not found,
> the preferred if-statement is:
> if (notice !== null)
Thanks for this remark.

I have changed the if statement to explicitly test agains null.

Same for the other cases.

[snip]
> >+ LPS.use('lp.pofile', function(Y) {
> > Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
> >- Y.on('domready', Y.lp.pofile.updateNotificationBox);
> >+ });
> >+
> >+ LPS.use('lp.pofile', function(Y) {
>
>
> Why are you calling LPS.use('lp.pofile', ...) repeatedly? This
> should have no effect unless there is something seriously wrong with
> lp.pofile.
 I moved them back in the same sandbox.
I split independent functions in different sandboxes so that the failure of one function to not affect the others.

Here is the latest diff:

=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-17 13:11:23 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-17 22:18:24 +0000
@@ -59,7 +59,7 @@

 self.updateNotificationBox = function(e) {
   var notice = Y.one('.important-notice-container');
- if (!notice) {
+ if (notice == null) {
     // We have no notice container on this page, this is why there is
     // nothing more to be done by this function.
     return;
@@ -71,7 +71,7 @@
   // Check the cookie to see if the user has already dismissed
   // the notification box for this session.
   var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean);
- if (already_seen) {
+ if (already_seen !== null) {
      notice.setStyle('display', 'none');
   }

@@ -79,7 +79,7 @@
       '.important-notice-cancel-button');
   // Cancel button starts out hidden. If user has JavaScript,
   // then we want to show it.
- if (!cancel_button) {
+ if (cancel_button == null) {
     // No cancel button was found to attach the action.
     return;
   }
@@ -94,7 +94,7 @@

 self.setFocus = function(field) {
     // if there is nofield, do nothing
- if (Y.one('#' + field)) {
+ if (Y.one('#' + field) !== null) {
         Y.one('#' + field).focus();
     }
 };
@@ -151,7 +151,7 @@
         original_singular, translation_singular, singular_select);

     // Copy plural text if needed
- if (Y.one('#' + translation_plural + '1')) {
+ if (Y.one('#' + translation_plural + '1') !== null) {
         copyOrigi...

Read more...

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> > Why are you calling LPS.use('lp.pofile', ...) repeatedly? This
> > should have no effect unless there is something seriously wrong with
> > lp.pofile.
> I moved them back in the same sandbox.
> I split independent functions in different sandboxes so that the failure of
> one function to not affect the others.

Hi Adi,

All your changes look good. Now I understand what you were trying to do with the separate LPS calls. Since I wasn't completely certain how far up the stack an exception goes before it continues running the rest of the javascript in the page, I played around with it a little bit. Apparently, an exception halts execution of a <script> block, so you would have to have multiple <script> blocks around each LPS.use() in order to ensure that the following code is run despite an exception above it. A much clearer way to handle that would be:

LPS.use('lp.pofile', function(Y) {
  try {
    do_something();
  } catch (e) {
    Y.log(e, "error");
  }
  do_something_else();
});

At least in firefox, if you call Y.log() with the second argument as "error", you will get a full stack trace in the console just like a normal uncaught exception.

Using try/catch is completely up to you.

 merge-approved

BTW, calling LPS.use() multiple times does not give you separate sandboxes, since you are re-using LPS instead of creating a new YUI() object.

-Edwin

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

I added try/catch statements.

Many thanks for your review.

Do you have time to send this branch to ec2 test and land, or should I ask the ORC?

Here is the latest diff.
=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
--- lib/lp/translations/templates/pofile-translate.pt 2010-03-17 23:17:46 +0000
+++ lib/lp/translations/templates/pofile-translate.pt 2010-03-18 16:26:48 +0000
@@ -18,12 +18,28 @@
       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");
+ }
       });

=== modified file 'lib/lp/translations/templates/translationmessage-translate.pt'
--- lib/lp/translations/templates/translationmessage-translate.pt 2010-03-17 23:17:46 +0000
+++ lib/lp/translations/templates/translationmessage-translate.pt 2010-03-18 16:25:45 +0000
@@ -15,11 +15,23 @@
     </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");
+ }
       });
     </script>
     </div>

Revision history for this message
Adi Roiban (adiroiban) wrote :
Download full text (3.6 KiB)

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");
...

Read more...

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

Hi Edwin and thanks for the review!

I moved the try/catch in a single domready block, declared link as a local varibaled and used === to compare for null.

Here is the latests diff:
=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-22 14:59:40 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-22 17:43:04 +0000
@@ -59,7 +59,7 @@

 self.updateNotificationBox = function(e) {
   var notice = Y.one('.important-notice-container');
- if (notice == null) {
+ if (notice === null) {
     // We have no notice container on this page, this is why there is
     // nothing more to be done by this function.
     return;
@@ -79,7 +79,7 @@
       '.important-notice-cancel-button');
   // Cancel button starts out hidden. If user has JavaScript,
   // then we want to show it.
- if (cancel_button == null) {
+ if (cancel_button === null) {
     // No cancel button was found to attach the action.
     return;
   }
@@ -197,6 +197,7 @@
 var initializeGlobalKeyBindings = function(fields) {

     Y.get('document').on("keyup", function(e) {
+ var link;
         // Shift+Alt+s - Save form
         if (e.shiftKey && e.altKey && e.keyCode == 83) {
             Y.one('#save_and_continue_button').invoke('click');

=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
--- lib/lp/translations/templates/pofile-translate.pt 2010-03-22 14:59:44 +0000
+++ lib/lp/translations/templates/pofile-translate.pt 2010-03-22 17:34:27 +0000
@@ -25,25 +25,19 @@
             } 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) {

=== modified file 'lib/lp/translations/templates/translationmessage-translate.pt'
--- lib/lp/translations/templates/translationmessage-translate.pt 2010-03-22 14:50:08 +0000
+++ lib/lp/translations/templates/translationmessage-translate.pt 2010-03-22 17:35:02 +0000
@@ -21,17 +21,13 @@
             } 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) {

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-02-25 12:56:45 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-22 17:49:29 +0000
@@ -55,10 +55,15 @@
55 node.setStyle('display', 'none');55 node.setStyle('display', 'none');
56 });56 });
57 hide_anim.run();57 hide_anim.run();
58}58};
5959
60self.updateNotificationBox = function(e) {60self.updateNotificationBox = function(e) {
61 var notice = Y.one('.important-notice-container');61 var notice = Y.one('.important-notice-container');
62 if (notice === null) {
63 // We have no notice container on this page, this is why there is
64 // nothing more to be done by this function.
65 return;
66 }
62 var balloon = notice.one('.important-notice-balloon');67 var balloon = notice.one('.important-notice-balloon');
63 var dismiss_notice_cookie = ('translation-docs-for-' +68 var dismiss_notice_cookie = ('translation-docs-for-' +
64 documentation_cookie);69 documentation_cookie);
@@ -66,7 +71,7 @@
66 // Check the cookie to see if the user has already dismissed71 // Check the cookie to see if the user has already dismissed
67 // the notification box for this session.72 // the notification box for this session.
68 var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean);73 var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean);
69 if (already_seen) {74 if (already_seen !== null) {
70 notice.setStyle('display', 'none');75 notice.setStyle('display', 'none');
71 }76 }
7277
@@ -74,6 +79,10 @@
74 '.important-notice-cancel-button');79 '.important-notice-cancel-button');
75 // Cancel button starts out hidden. If user has JavaScript,80 // Cancel button starts out hidden. If user has JavaScript,
76 // then we want to show it.81 // then we want to show it.
82 if (cancel_button === null) {
83 // No cancel button was found to attach the action.
84 return;
85 }
77 cancel_button.setStyle('visibility', 'visible');86 cancel_button.setStyle('visibility', 'visible');
78 cancel_button.on('click', function(e) {87 cancel_button.on('click', function(e) {
79 e.halt();88 e.halt();
@@ -85,7 +94,7 @@
8594
86self.setFocus = function(field) {95self.setFocus = function(field) {
87 // if there is nofield, do nothing96 // if there is nofield, do nothing
88 if (Y.one('#' + field)) {97 if (Y.one('#' + field) !== null) {
89 Y.one('#' + field).focus();98 Y.one('#' + field).focus();
90 }99 }
91};100};
@@ -142,7 +151,7 @@
142 original_singular, translation_singular, singular_select);151 original_singular, translation_singular, singular_select);
143152
144 // Copy plural text if needed153 // Copy plural text if needed
145 if (Y.one('#' + translation_plural + '1')) {154 if (Y.one('#' + translation_plural + '1') !== null) {
146 copyOriginalTextPlural(155 copyOriginalTextPlural(
147 original_plural, translation_plural, plural_forms);156 original_plural, translation_plural, plural_forms);
148 }157 }
@@ -153,7 +162,7 @@
153162
154var selectWidgetByID = function(widget) {163var selectWidgetByID = function(widget) {
155 var node = Y.one('#' + widget);164 var node = Y.one('#' + widget);
156 if (node) {165 if (node !== null) {
157 node.set('checked', true);166 node.set('checked', true);
158 }167 }
159};168};
@@ -161,7 +170,7 @@
161170
162var toggleWidget = function(widget) {171var toggleWidget = function(widget) {
163 var node = Y.one('#' + widget);172 var node = Y.one('#' + widget);
164 if (node) {173 if (node !== null) {
165 if (node.get('checked')) {174 if (node.get('checked')) {
166 node.set('checked', false);175 node.set('checked', false);
167 } else {176 } else {
@@ -187,45 +196,50 @@
187196
188var initializeGlobalKeyBindings = function(fields) {197var initializeGlobalKeyBindings = function(fields) {
189198
190 Y.get('document').on("keyup", function(e) {199 Y.get('document').on("keyup", function(e) {
200 var link;
191 // Shift+Alt+s - Save form201 // Shift+Alt+s - Save form
192 if (e.shiftKey && e.altKey && e.keyCode == 83) {202 if (e.shiftKey && e.altKey && e.keyCode == 83) {
193 Y.one('#save_and_continue_button').invoke('click');203 Y.one('#save_and_continue_button').invoke('click');
194 }204 }
195 // Shift+Alt+f - Go to search field205 // Shift+Alt+f - Go to search field
196 if (e.shiftKey && e.altKey && e.keyCode == 70) {206 if (e.shiftKey && e.altKey && e.keyCode == 70) {
197 self.setFocus('search_box');207 self.setFocus('search_box');
198 }208 }
199 // Shift+Alt+b - Go to first translation field209 // Shift+Alt+b - Go to first translation field
200 if (e.shiftKey && e.altKey && e.keyCode == 66) {210 if (e.shiftKey && e.altKey && e.keyCode == 66) {
201 self.setFocus(fields[0]);211 self.setFocus(fields[0]);
202 }212 }
203 // Shift+Alt+n - Go to next page in batch213 // Shift+Alt+n - Go to next page in batch
204 if (e.shiftKey && e.altKey && e.keyCode == 78) {214 if (e.shiftKey && e.altKey && e.keyCode == 78) {
205 if (link = Y.one('#batchnav_next')){215 link = Y.one('#batchnav_next');
206 window.location.assign(link.get('href'))216 if (link !== null){
207 }217 window.location.assign(link.get('href'));
208 }218 }
219 }
209 // Shift+Alt+p - Go to previous page in batch220 // Shift+Alt+p - Go to previous page in batch
210 if (e.shiftKey && e.altKey && e.keyCode == 80) {221 if (e.shiftKey && e.altKey && e.keyCode == 80) {
211 if (link = Y.one('#batchnav_previous')){222 link = Y.one('#batchnav_previous');
212 window.location.assign(link.get('href'))223 if (link !== null){
213 }224 window.location.assign(link.get('href'));
214 }225 }
226 }
215 // Shift+Alt+a - Go to first page in batch227 // Shift+Alt+a - Go to first page in batch
216 if (e.shiftKey && e.altKey && e.keyCode == 65) {228 if (e.shiftKey && e.altKey && e.keyCode == 65) {
217 if (link = Y.one('#batchnav_first')){229 link = Y.one('#batchnav_first');
218 window.location.assign(link.get('href'))230 if (link !== null){
219 }231 window.location.assign(link.get('href'));
220 }232 }
233 }
221 // Shift+Alt+l - Go to last page in batch234 // Shift+Alt+l - Go to last page in batch
222 if (e.shiftKey && e.altKey && e.keyCode == 76) {235 if (e.shiftKey && e.altKey && e.keyCode == 76) {
223 if (link = Y.one('#batchnav_last')){236 link = Y.one('#batchnav_last');
224 window.location.assign(link.get('href'))237 if (link !== null){
225 }238 window.location.assign(link.get('href'));
226 }239 }
227 });240 }
228}241 });
242};
229243
230244
231var initializeSuggestionsKeyBindings = function(stem) {245var initializeSuggestionsKeyBindings = function(stem) {
@@ -243,7 +257,7 @@
243 Y, node.get('id'));257 Y, node.get('id'));
244 }258 }
245 });259 });
246}260};
247261
248262
249var initializeFieldsKeyBindings = function (fields) {263var initializeFieldsKeyBindings = function (fields) {
@@ -321,7 +335,7 @@
321 singular_copy_text);335 singular_copy_text);
322 }336 }
323 }337 }
324}338};
325339
326340
327/**341/**
328342
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2010-03-15 17:17:05 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2010-03-22 17:49:29 +0000
@@ -211,6 +211,9 @@
211 <script type="text/javascript"211 <script type="text/javascript"
212 tal:attributes="src string:${lp_js}/registry/team.js"></script>212 tal:attributes="src string:${lp_js}/registry/team.js"></script>
213213
214 <script type="text/javascript"
215 tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
216
214 </tal:devmode>217 </tal:devmode>
215 <tal:production condition="not:devmode">218 <tal:production condition="not:devmode">
216 <script type="text/javascript"219 <script type="text/javascript"
217220
=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
--- lib/lp/translations/templates/pofile-translate.pt 2010-02-25 00:47:17 +0000
+++ lib/lp/translations/templates/pofile-translate.pt 2010-03-22 17:49:29 +0000
@@ -14,21 +14,39 @@
14 }14 }
15 </style>15 </style>
1616
17 <script type="text/javascript"
18 tal:condition="devmode"
19 tal:define="lp_js string:${icingroot}/build"
20 tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
21 <script type="text/javascript">17 <script type="text/javascript">
22 registerLaunchpadFunction(insertAllExpansionButtons);18 registerLaunchpadFunction(insertAllExpansionButtons);
2319
24 LPS.use('lp.pofile', function(Y) {20 LPS.use('lp.pofile', function(Y) {
25 Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);21
26 Y.on('domready', Y.lp.pofile.updateNotificationBox);
27 Y.on('domready', Y.lp.pofile.initializeKeyBindings);
28 Y.on('domready', function(e) {22 Y.on('domready', function(e) {
29 Y.lp.pofile.setFocus(autofocus_field);23 try {
24 Y.lp.pofile.updateNotificationBox();
25 } catch (e) {
26 Y.log(e, "error");
27 }
28
29 try {
30 Y.lp.pofile.setupSuggestionDismissal();
31 } catch (e) {
32 Y.log(e, "error");
33 }
34
35 try {
36 Y.lp.pofile.initializeKeyBindings();
37 } catch (e) {
38 Y.log(e, "error");
39 }
40
41 try {
42 Y.lp.pofile.setFocus(autofocus_field);
43 } catch (e) {
44 Y.log(e, "error");
45 }
30 });46 });
47
31 });48 });
49
32 </script>50 </script>
33 </div>51 </div>
34 <div metal:fill-slot="main">52 <div metal:fill-slot="main">
3553
=== modified file 'lib/lp/translations/templates/translationmessage-translate.pt'
--- lib/lp/translations/templates/translationmessage-translate.pt 2010-02-25 00:47:17 +0000
+++ lib/lp/translations/templates/translationmessage-translate.pt 2010-03-22 17:49:29 +0000
@@ -13,16 +13,26 @@
13 color: lightgray;13 color: lightgray;
14 }14 }
15 </style>15 </style>
16 <script type="text/javascript"
17 tal:condition="devmode"
18 tal:define="lp_js string:${icingroot}/build"
19 tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
20 <script type="text/javascript">16 <script type="text/javascript">
21 LPS.use('node', 'lp.pofile', function(Y) {17 LPS.use('node', 'lp.pofile', function(Y) {
22 Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
23 Y.on('domready', Y.lp.pofile.initializeKeyBindings);
24 Y.on('domready', function(e) {18 Y.on('domready', function(e) {
25 Y.lp.pofile.setFocus(autofocus_field);19 try {
20 Y.lp.pofile.setupSuggestionDismissal();
21 } catch (e) {
22 Y.log(e, "error");
23 }
24
25 try {
26 Y.lp.pofile.initializeKeyBindings();
27 } catch (e) {
28 Y.log(e, "error");
29 }
30
31 try {
32 Y.lp.pofile.setFocus(autofocus_field);
33 } catch (e) {
34 Y.log(e, "error");
35 }
26 });36 });
27 });37 });
28 </script>38 </script>
2939
=== added file 'lib/lp/translations/windmill/tests/test_pofile_translate.py'
--- lib/lp/translations/windmill/tests/test_pofile_translate.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-03-22 17:49:29 +0000
@@ -0,0 +1,52 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for pofile translate pages."""
5
6__metaclass__ = type
7__all__ = []
8
9from canonical.launchpad.windmill.testing import constants, lpuser
10from lp.translations.windmill.testing import TranslationsWindmillLayer
11from lp.testing import WindmillTestCase
12
13
14class POFileNewTranslationFieldKeybindings(WindmillTestCase):
15 """Tests for keybinding actions associated to the translation field."""
16
17 layer = TranslationsWindmillLayer
18 suite_name = 'POFile Translate'
19
20 def test_pofile_new_translation_autoselect(self):
21 """Test for automatically selecting new translation on text input.
22
23 When new text is typed into the new translation text fields, the
24 associated radio button should be automatically selected.
25 """
26 client = self.client
27 start_url = ('http://translations.launchpad.dev:8085/'
28 'evolution/trunk/+pots/evolution-2.2/es/+translate')
29 user = lpuser.TRANSLATIONS_ADMIN
30 new_translation_id = u'msgset_1_es_translation_0_new'
31 radiobutton_id = u'msgset_1_es_translation_0_new_select'
32
33 # Go to the translation page.
34 self.client.open(url=start_url)
35 self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
36 user.ensure_login(self.client)
37
38 # Wait for the new translation field and it's associated radio button.
39 client.waits.forElement(
40 id=new_translation_id, timeout=constants.FOR_ELEMENT)
41 client.waits.forElement(
42 id=radiobutton_id, timeout=constants.FOR_ELEMENT)
43
44 # Check that the associated radio button is not selected.
45 client.asserts.assertNotChecked(id=radiobutton_id)
46
47 # Type a new translation.
48 client.type(
49 id=new_translation_id, text=u'New translation')
50
51 # Check that the associated radio button is selected.
52 client.asserts.assertChecked(id=radiobutton_id)