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
1=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
2--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-02-25 12:56:45 +0000
3+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-22 17:49:29 +0000
4@@ -55,10 +55,15 @@
5 node.setStyle('display', 'none');
6 });
7 hide_anim.run();
8-}
9+};
10
11 self.updateNotificationBox = function(e) {
12 var notice = Y.one('.important-notice-container');
13+ if (notice === null) {
14+ // We have no notice container on this page, this is why there is
15+ // nothing more to be done by this function.
16+ return;
17+ }
18 var balloon = notice.one('.important-notice-balloon');
19 var dismiss_notice_cookie = ('translation-docs-for-' +
20 documentation_cookie);
21@@ -66,7 +71,7 @@
22 // Check the cookie to see if the user has already dismissed
23 // the notification box for this session.
24 var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean);
25- if (already_seen) {
26+ if (already_seen !== null) {
27 notice.setStyle('display', 'none');
28 }
29
30@@ -74,6 +79,10 @@
31 '.important-notice-cancel-button');
32 // Cancel button starts out hidden. If user has JavaScript,
33 // then we want to show it.
34+ if (cancel_button === null) {
35+ // No cancel button was found to attach the action.
36+ return;
37+ }
38 cancel_button.setStyle('visibility', 'visible');
39 cancel_button.on('click', function(e) {
40 e.halt();
41@@ -85,7 +94,7 @@
42
43 self.setFocus = function(field) {
44 // if there is nofield, do nothing
45- if (Y.one('#' + field)) {
46+ if (Y.one('#' + field) !== null) {
47 Y.one('#' + field).focus();
48 }
49 };
50@@ -142,7 +151,7 @@
51 original_singular, translation_singular, singular_select);
52
53 // Copy plural text if needed
54- if (Y.one('#' + translation_plural + '1')) {
55+ if (Y.one('#' + translation_plural + '1') !== null) {
56 copyOriginalTextPlural(
57 original_plural, translation_plural, plural_forms);
58 }
59@@ -153,7 +162,7 @@
60
61 var selectWidgetByID = function(widget) {
62 var node = Y.one('#' + widget);
63- if (node) {
64+ if (node !== null) {
65 node.set('checked', true);
66 }
67 };
68@@ -161,7 +170,7 @@
69
70 var toggleWidget = function(widget) {
71 var node = Y.one('#' + widget);
72- if (node) {
73+ if (node !== null) {
74 if (node.get('checked')) {
75 node.set('checked', false);
76 } else {
77@@ -187,45 +196,50 @@
78
79 var initializeGlobalKeyBindings = function(fields) {
80
81- Y.get('document').on("keyup", function(e) {
82+ Y.get('document').on("keyup", function(e) {
83+ var link;
84 // Shift+Alt+s - Save form
85- if (e.shiftKey && e.altKey && e.keyCode == 83) {
86- Y.one('#save_and_continue_button').invoke('click');
87- }
88+ if (e.shiftKey && e.altKey && e.keyCode == 83) {
89+ Y.one('#save_and_continue_button').invoke('click');
90+ }
91 // Shift+Alt+f - Go to search field
92- if (e.shiftKey && e.altKey && e.keyCode == 70) {
93- self.setFocus('search_box');
94- }
95+ if (e.shiftKey && e.altKey && e.keyCode == 70) {
96+ self.setFocus('search_box');
97+ }
98 // Shift+Alt+b - Go to first translation field
99- if (e.shiftKey && e.altKey && e.keyCode == 66) {
100- self.setFocus(fields[0]);
101- }
102+ if (e.shiftKey && e.altKey && e.keyCode == 66) {
103+ self.setFocus(fields[0]);
104+ }
105 // Shift+Alt+n - Go to next page in batch
106- if (e.shiftKey && e.altKey && e.keyCode == 78) {
107- if (link = Y.one('#batchnav_next')){
108- window.location.assign(link.get('href'))
109- }
110- }
111+ if (e.shiftKey && e.altKey && e.keyCode == 78) {
112+ link = Y.one('#batchnav_next');
113+ if (link !== null){
114+ window.location.assign(link.get('href'));
115+ }
116+ }
117 // Shift+Alt+p - Go to previous page in batch
118- if (e.shiftKey && e.altKey && e.keyCode == 80) {
119- if (link = Y.one('#batchnav_previous')){
120- window.location.assign(link.get('href'))
121- }
122- }
123+ if (e.shiftKey && e.altKey && e.keyCode == 80) {
124+ link = Y.one('#batchnav_previous');
125+ if (link !== null){
126+ window.location.assign(link.get('href'));
127+ }
128+ }
129 // Shift+Alt+a - Go to first page in batch
130- if (e.shiftKey && e.altKey && e.keyCode == 65) {
131- if (link = Y.one('#batchnav_first')){
132- window.location.assign(link.get('href'))
133- }
134- }
135+ if (e.shiftKey && e.altKey && e.keyCode == 65) {
136+ link = Y.one('#batchnav_first');
137+ if (link !== null){
138+ window.location.assign(link.get('href'));
139+ }
140+ }
141 // Shift+Alt+l - Go to last page in batch
142- if (e.shiftKey && e.altKey && e.keyCode == 76) {
143- if (link = Y.one('#batchnav_last')){
144- window.location.assign(link.get('href'))
145- }
146- }
147- });
148-}
149+ if (e.shiftKey && e.altKey && e.keyCode == 76) {
150+ link = Y.one('#batchnav_last');
151+ if (link !== null){
152+ window.location.assign(link.get('href'));
153+ }
154+ }
155+ });
156+};
157
158
159 var initializeSuggestionsKeyBindings = function(stem) {
160@@ -243,7 +257,7 @@
161 Y, node.get('id'));
162 }
163 });
164-}
165+};
166
167
168 var initializeFieldsKeyBindings = function (fields) {
169@@ -321,7 +335,7 @@
170 singular_copy_text);
171 }
172 }
173-}
174+};
175
176
177 /**
178
179=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
180--- lib/lp/app/templates/base-layout-macros.pt 2010-03-15 17:17:05 +0000
181+++ lib/lp/app/templates/base-layout-macros.pt 2010-03-22 17:49:29 +0000
182@@ -211,6 +211,9 @@
183 <script type="text/javascript"
184 tal:attributes="src string:${lp_js}/registry/team.js"></script>
185
186+ <script type="text/javascript"
187+ tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
188+
189 </tal:devmode>
190 <tal:production condition="not:devmode">
191 <script type="text/javascript"
192
193=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
194--- lib/lp/translations/templates/pofile-translate.pt 2010-02-25 00:47:17 +0000
195+++ lib/lp/translations/templates/pofile-translate.pt 2010-03-22 17:49:29 +0000
196@@ -14,21 +14,39 @@
197 }
198 </style>
199
200- <script type="text/javascript"
201- tal:condition="devmode"
202- tal:define="lp_js string:${icingroot}/build"
203- tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
204 <script type="text/javascript">
205 registerLaunchpadFunction(insertAllExpansionButtons);
206
207 LPS.use('lp.pofile', function(Y) {
208- Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
209- Y.on('domready', Y.lp.pofile.updateNotificationBox);
210- Y.on('domready', Y.lp.pofile.initializeKeyBindings);
211+
212 Y.on('domready', function(e) {
213- Y.lp.pofile.setFocus(autofocus_field);
214+ try {
215+ Y.lp.pofile.updateNotificationBox();
216+ } catch (e) {
217+ Y.log(e, "error");
218+ }
219+
220+ try {
221+ Y.lp.pofile.setupSuggestionDismissal();
222+ } catch (e) {
223+ Y.log(e, "error");
224+ }
225+
226+ try {
227+ Y.lp.pofile.initializeKeyBindings();
228+ } catch (e) {
229+ Y.log(e, "error");
230+ }
231+
232+ try {
233+ Y.lp.pofile.setFocus(autofocus_field);
234+ } catch (e) {
235+ Y.log(e, "error");
236+ }
237 });
238+
239 });
240+
241 </script>
242 </div>
243 <div metal:fill-slot="main">
244
245=== modified file 'lib/lp/translations/templates/translationmessage-translate.pt'
246--- lib/lp/translations/templates/translationmessage-translate.pt 2010-02-25 00:47:17 +0000
247+++ lib/lp/translations/templates/translationmessage-translate.pt 2010-03-22 17:49:29 +0000
248@@ -13,16 +13,26 @@
249 color: lightgray;
250 }
251 </style>
252- <script type="text/javascript"
253- tal:condition="devmode"
254- tal:define="lp_js string:${icingroot}/build"
255- tal:attributes="src string:${lp_js}/translations/pofile.js"></script>
256 <script type="text/javascript">
257 LPS.use('node', 'lp.pofile', function(Y) {
258- Y.on('domready', Y.lp.pofile.setupSuggestionDismissal);
259- Y.on('domready', Y.lp.pofile.initializeKeyBindings);
260 Y.on('domready', function(e) {
261- Y.lp.pofile.setFocus(autofocus_field);
262+ try {
263+ Y.lp.pofile.setupSuggestionDismissal();
264+ } catch (e) {
265+ Y.log(e, "error");
266+ }
267+
268+ try {
269+ Y.lp.pofile.initializeKeyBindings();
270+ } catch (e) {
271+ Y.log(e, "error");
272+ }
273+
274+ try {
275+ Y.lp.pofile.setFocus(autofocus_field);
276+ } catch (e) {
277+ Y.log(e, "error");
278+ }
279 });
280 });
281 </script>
282
283=== added file 'lib/lp/translations/windmill/tests/test_pofile_translate.py'
284--- lib/lp/translations/windmill/tests/test_pofile_translate.py 1970-01-01 00:00:00 +0000
285+++ lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-03-22 17:49:29 +0000
286@@ -0,0 +1,52 @@
287+# Copyright 2010 Canonical Ltd. This software is licensed under the
288+# GNU Affero General Public License version 3 (see the file LICENSE).
289+
290+"""Tests for pofile translate pages."""
291+
292+__metaclass__ = type
293+__all__ = []
294+
295+from canonical.launchpad.windmill.testing import constants, lpuser
296+from lp.translations.windmill.testing import TranslationsWindmillLayer
297+from lp.testing import WindmillTestCase
298+
299+
300+class POFileNewTranslationFieldKeybindings(WindmillTestCase):
301+ """Tests for keybinding actions associated to the translation field."""
302+
303+ layer = TranslationsWindmillLayer
304+ suite_name = 'POFile Translate'
305+
306+ def test_pofile_new_translation_autoselect(self):
307+ """Test for automatically selecting new translation on text input.
308+
309+ When new text is typed into the new translation text fields, the
310+ associated radio button should be automatically selected.
311+ """
312+ client = self.client
313+ start_url = ('http://translations.launchpad.dev:8085/'
314+ 'evolution/trunk/+pots/evolution-2.2/es/+translate')
315+ user = lpuser.TRANSLATIONS_ADMIN
316+ new_translation_id = u'msgset_1_es_translation_0_new'
317+ radiobutton_id = u'msgset_1_es_translation_0_new_select'
318+
319+ # Go to the translation page.
320+ self.client.open(url=start_url)
321+ self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
322+ user.ensure_login(self.client)
323+
324+ # Wait for the new translation field and it's associated radio button.
325+ client.waits.forElement(
326+ id=new_translation_id, timeout=constants.FOR_ELEMENT)
327+ client.waits.forElement(
328+ id=radiobutton_id, timeout=constants.FOR_ELEMENT)
329+
330+ # Check that the associated radio button is not selected.
331+ client.asserts.assertNotChecked(id=radiobutton_id)
332+
333+ # Type a new translation.
334+ client.type(
335+ id=new_translation_id, text=u'New translation')
336+
337+ # Check that the associated radio button is selected.
338+ client.asserts.assertChecked(id=radiobutton_id)