Launchpad itself

Merge lp:~benji/launchpad/better-HTML-generation into lp:launchpad

  1. better-HTML-generation
  2. Merge into devel
Proposed by Benji York
Status: Merged
Approved by: Benji York
Approved revision: no longer in the source branch.
Merged at revision: 12731
Proposed branch: lp:~benji/launchpad/better-HTML-generation
Merge into: lp:launchpad
Diff against target: 930 lines (+311/-290)
2 files modified
lib/lp/registry/javascript/structural-subscription.js (+301/-281)
lib/lp/registry/javascript/tests/test_structural_subscription.js (+10/-9)
To merge this branch: bzr merge lp:~benji/launchpad/better-HTML-generation
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+55960@code.launchpad.net

Commit message

[r=bac][no-qa] Clean up HTML generation, primarily to avoid XSS.

Description of the change

This branch fixes the way structural subscription JavaScript constructs HTML, moving away from string concatenation to Y.Node.create() and friends.

There's also a fair bit of lint fixing required to quite JSLint.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

This is a vast improvement over our original way of creating the html content for the overlay. Thanks for cleaning it all up.

As we discussed on IRC I am a little concerned that there are still large sections of string concatenation to create the recipient picker and overlay container. You pointed out it is all constant string values so it shouldn't present a problem. In the future we can always break those sections down if it proves to be needed.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js 2011-03-30 15:23:42 +0000
+++ lib/lp/registry/javascript/structural-subscription.js 2011-04-01 17:57:54 +0000
@@ -11,9 +11,6 @@
1111
12var namespace = Y.namespace('lp.registry.structural_subscription');12var namespace = Y.namespace('lp.registry.structural_subscription');
1313
14var INNER_HTML = 'innerHTML',
15 VALUE = 'value';
16
17var FILTER_COMMENTS = 'filter-comments',14var FILTER_COMMENTS = 'filter-comments',
18 FILTER_WRAPPER = 'filter-wrapper',15 FILTER_WRAPPER = 'filter-wrapper',
19 ACCORDION_WRAPPER = 'accordion-wrapper',16 ACCORDION_WRAPPER = 'accordion-wrapper',
@@ -21,8 +18,7 @@
21 ADDED_OR_CHANGED = 'added-or-changed',18 ADDED_OR_CHANGED = 'added-or-changed',
22 ADVANCED_FILTER = 'advanced-filter',19 ADVANCED_FILTER = 'advanced-filter',
23 MATCH_ALL = 'match-all',20 MATCH_ALL = 'match-all',
24 MATCH_ANY = 'match-any',21 MATCH_ANY = 'match-any'
25 SS_COLLAPSIBLE = 'ss-collapsible'
26 ;22 ;
2723
28var add_subscription_overlay;24var add_subscription_overlay;
@@ -61,7 +57,7 @@
61function list_contains(list, target) {57function list_contains(list, target) {
62 // The list may be undefined in some cases.58 // The list may be undefined in some cases.
63 return Y.Lang.isArray(list) && list.indexOf(target) !== -1;59 return Y.Lang.isArray(list) && list.indexOf(target) !== -1;
64};60}
6561
66// Expose to tests.62// Expose to tests.
67namespace._list_contains = list_contains;63namespace._list_contains = list_contains;
@@ -209,8 +205,9 @@
209function save_subscription(form_data) {205function save_subscription(form_data) {
210 var who;206 var who;
211 var has_errors = check_for_errors_in_overlay(add_subscription_overlay);207 var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
212 if (has_errors)208 if (has_errors) {
213 return false;209 return false;
210 }
214 if (form_data.recipient[0] === 'user') {211 if (form_data.recipient[0] === 'user') {
215 who = LP.links.me;212 who = LP.links.me;
216 } else {213 } else {
@@ -223,15 +220,18 @@
223220
224function check_for_errors_in_overlay(overlay) {221function check_for_errors_in_overlay(overlay) {
225 var has_errors = false;222 var has_errors = false;
226 var errors = new Array();223 var errors = [];
227 for (var field in overlay.field_errors) {224 var field;
228 if (overlay.field_errors[field]) {225 for (field in overlay.field_errors) {
229 has_errors = true;226 if (overlay.field_errors.hasOwnProperty(field)) {
230 errors.push(field);227 if (overlay.field_errors[field]) {
228 has_errors = true;
229 errors.push(field);
230 }
231 }231 }
232 };232 }
233 if (has_errors) {233 if (has_errors) {
234 var error_text = errors.pop()234 var error_text = errors.pop();
235 if (errors.length > 0) {235 if (errors.length > 0) {
236 error_text = errors.join(', ') + ' and ' + error_text;236 error_text = errors.join(', ') + ' and ' + error_text;
237 }237 }
@@ -242,40 +242,38 @@
242 } else {242 } else {
243 return false;243 return false;
244 }244 }
245};245}
246246
247/**247/**
248 * Handle the activation of the edit subscription link.248 * Handle the activation of the edit subscription link.
249 */249 */
250function edit_subscription_handler(context, form_data) {250function edit_subscription_handler(context, form_data) {
251 var has_errors = check_for_errors_in_overlay(add_subscription_overlay);251 var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
252 if (has_errors)252 var filter_id = '#filter-description-'+context.filter_id.toString();
253 if (has_errors) {
253 return false;254 return false;
255 }
254 var on = {success: function (new_data) {256 var on = {success: function (new_data) {
255 var filter = new_data.getAttrs();257 var filter = new_data.getAttrs();
256 var description_node = Y.one(258 var description_node = Y.one(filter_id);
257 '#filter-description-'+context.filter_id.toString());259 description_node
258 description_node.set(260 .empty()
259 INNER_HTML, render_filter_description(filter));261 .appendChild(create_filter_description(filter));
260 var name_node = Y.one(262 description_node.ancestor('.subscription-filter').one('.filter-name')
261 '#filter-name-'+context.filter_id.toString());263 .empty()
262 name_node.set(264 .appendChild(render_filter_title(context.filter_info, filter));
263 INNER_HTML, render_filter_name(context.filter_info, filter));
264 add_subscription_overlay.hide();265 add_subscription_overlay.hide();
265 }};266 }};
266 patch_bug_filter(context.filter_info.filter, form_data, on);267 patch_bug_filter(context.filter_info.filter, form_data, on);
267}268}
268269
270/**
271 * Initialize the overlay errors and set up field validators.
272 */
269function setup_overlay_validators(overlay, overlay_id) {273function setup_overlay_validators(overlay, overlay_id) {
270 overlay.field_validators = {
271 tags: get_error_for_tags_list
272 };
273 overlay.field_errors = {};274 overlay.field_errors = {};
274 for (var field_name in overlay.field_validators) {275 add_input_validator(overlay, overlay_id, 'tags', get_error_for_tags_list);
275 add_input_validator(overlay, overlay_id, field_name,276}
276 overlay.field_validators[field_name]);
277 };
278};
279277
280/**278/**
281 * Populate the overlay element with the contents of the add/edit form.279 * Populate the overlay element with the contents of the add/edit form.
@@ -295,8 +293,9 @@
295 form_submit_callback: function(formdata) {293 form_submit_callback: function(formdata) {
296 // Do not clean up if saving was not successful.294 // Do not clean up if saving was not successful.
297 var save_succeeded = submit_callback(formdata);295 var save_succeeded = submit_callback(formdata);
298 if (save_succeeded !== false)296 if (save_succeeded !== false) {
299 clean_up();297 clean_up();
298 }
300 }299 }
301 });300 });
302 add_subscription_overlay.render(content_box_id);301 add_subscription_overlay.render(content_box_id);
@@ -373,10 +372,14 @@
373 * @param {String} name Name of the control.372 * @param {String} name Name of the control.
374 */373 */
375function make_cell(item, name) {374function make_cell(item, name) {
376 return '<td style="padding-left:3px"><label><input type="checkbox" ' +375 var cell = Y.Node.create('<td style="padding-left:3px"><td>');
377 'name="' + name +'" ' +376 cell.appendChild('<label></label>')
378 'value="' + item + '" checked="checked">' +377 .append(Y.Node.create('<input type="checkbox" checked></input>')
379 item + '</label><td>';378 .set('name', name)
379 .set('value', item))
380 .append(Y.Node.create('<span></span>')
381 .set('text', item));
382 return cell;
380}383}
381/**384/**
382 * Make a table.385 * Make a table.
@@ -388,19 +391,15 @@
388 * @param {Int} num_cols The number of columns for the table to use.391 * @param {Int} num_cols The number of columns for the table to use.
389 */392 */
390function make_table(list, name, num_cols) {393function make_table(list, name, num_cols) {
391 var html = '<table>';394 var table = Y.Node.create('<table></table>');
392 var i;395 var i, row;
393 for (i=0; i<list.length; i++) {396 for (i=0; i<list.length; i++) {
394 if (i % num_cols === 0) {397 if (i % num_cols === 0) {
395 if (i !== 0) {398 row = table.appendChild('<tr></tr>');
396 html += '</tr>';
397 }
398 html += '<tr>';
399 }399 }
400 html += make_cell(list[i], name);400 row.appendChild(make_cell(list[i], name));
401 }401 }
402 html += '</tr></table>';402 return table;
403 return html;
404}403}
405404
406/**405/**
@@ -413,16 +412,18 @@
413 * @return {Object} Hash with 'all_name', 'none_name', and 'html' keys.412 * @return {Object} Hash with 'all_name', 'none_name', and 'html' keys.
414 */413 */
415function make_selector_controls(parent) {414function make_selector_controls(parent) {
415 var selectors_id = parent + '-selectors';
416 var rv = {};416 var rv = {};
417 rv.all_name = parent + '-select-all';417 rv.all_link = Y.Node.create(
418 rv.none_name = parent + '-select-none';418 '<a href="#" class="select-all">Select all</a>');
419 rv.html = '<div id="'+ parent + '-selectors" '+419 rv.none_link = Y.Node.create(
420 'style="margin-left: 10px;margin-bottom: 10px">' +420 '<a href="#" class="select-none">Select none</a>');
421 ' <a href="#" id="' + rv.all_name +421 rv.node = Y.Node.create(
422 '">Select all</a> &nbsp;' +422 '<div style="margin-left: 10px;margin-bottom: 10px"></div>')
423 ' <a href="#" id="' + rv.none_name +423 .set('id', selectors_id)
424 '">Select none</a>' +424 .append(rv.all_link)
425 '</div>';425 .append(' &nbsp; ')
426 .append(rv.none_link);
426427
427 return rv;428 return rv;
428}429}
@@ -474,27 +475,23 @@
474 id: "tags_ai",475 id: "tags_ai",
475 contentHeight: {method: "auto"}476 contentHeight: {method: "auto"}
476 } );477 } );
477
478 tags_container = tags_ai;478 tags_container = tags_ai;
479479 tags_ai.set("bodyContent", Y.Node.create('<div><div></div></div>')
480 tags_ai.set("bodyContent",480 .append(Y.Node.create('<input type="radio" name="tag_match" checked>')
481 '<div>\n' +481 .set('value', MATCH_ALL))
482 '<div>\n' +482 .append('Match all tags')
483 ' <input type="radio" name="tag_match" value="' +483 .append(Y.Node.create('<input type="radio" name="tag_match" checked>')
484 MATCH_ALL + '" checked> Match all tags\n' +484 .set('value', MATCH_ANY))
485 ' <input type="radio" name="tag_match" value="' +485 .append('Match any tags')
486 MATCH_ANY + '"> Match any tags\n' +486 .append(
487 '</div>\n' +487 '<div style="padding-bottom:10px;">' +
488 '<div style="padding-bottom:10px;">\n' +488 ' <input type="text" name="tags" size="60"/>' +
489 ' <input type="text" name="tags" size="60"/>\n' +
490 ' <a target="help"'+489 ' <a target="help"'+
491 ' href="/+help/structural-subscription-tags.html" ' +490 ' href="/+help/structural-subscription-tags.html" ' +
492 ' class="sprite maybe">&nbsp;'+491 ' class="sprite maybe">&nbsp;'+
493 '<span class="invisible-link">Structural subscription tags '+492 '<span class="invisible-link">Structural subscription tags '+
494 ' help</span></a>\n ' +493 ' help</span></a> ' +
495 '</div>\n' +494 '</div>'));
496 '</div>\n');
497
498 accordion.addItem(tags_ai);495 accordion.addItem(tags_ai);
499496
500 // Build importances pane.497 // Build importances pane.
@@ -507,20 +504,17 @@
507 } );504 } );
508 var importances = LP.cache.importances;505 var importances = LP.cache.importances;
509 var selectors = make_selector_controls('importances');506 var selectors = make_selector_controls('importances');
510 var importances_html = '<div id="importances-wrapper">' +507 importances_ai.set("bodyContent",
511 selectors.html +508 Y.Node.create('<div id="importances-wrapper"></div>')
512 make_table(importances, 'importances', 4) +509 .append(selectors.node)
513 '</div>';510 .append(make_table(importances, 'importances', 4)));
514 importances_ai.set("bodyContent", importances_html);
515 accordion.addItem(importances_ai);511 accordion.addItem(importances_ai);
516 // Wire up the 'all' and 'none' selectors.512 // Wire up the 'all' and 'none' selectors.
517 var all_link = content_node.one('#' + selectors.all_name);
518 var none_link = Y.one('#' + selectors.none_name);
519 var node = content_node.one('#importances-wrapper');513 var node = content_node.one('#importances-wrapper');
520 var select_all_handler = make_select_handler(node, importances, true);514 selectors.all_link.on('click',
521 var select_none_handler = make_select_handler(node, importances, false);515 make_select_handler(node, importances, true));
522 all_link.on('click', select_all_handler);516 selectors.none_link.on('click',
523 none_link.on('click', select_none_handler);517 make_select_handler(node, importances, false));
524518
525 // Build statuses pane.519 // Build statuses pane.
526 statuses_ai = new Y.AccordionItem( {520 statuses_ai = new Y.AccordionItem( {
@@ -532,18 +526,17 @@
532 } );526 } );
533 var statuses = LP.cache.statuses;527 var statuses = LP.cache.statuses;
534 selectors = make_selector_controls('statuses');528 selectors = make_selector_controls('statuses');
535 var status_html = '<div id="statuses-wrapper">' +529 statuses_ai.set("bodyContent",
536 selectors.html + make_table(statuses, 'statuses', 3)+530 Y.Node.create('<div id="statuses-wrapper"></div>')
537 '</div>';531 .append(selectors.node)
538 statuses_ai.set("bodyContent", status_html);532 .append(make_table(statuses, 'statuses', 3)));
539 accordion.addItem(statuses_ai);533 accordion.addItem(statuses_ai);
540 all_link = content_node.one('#' + selectors.all_name);534 // Wire up the 'all' and 'none' selectors.
541 none_link = Y.one('#' + selectors.none_name);
542 node = content_node.one('#statuses-wrapper');535 node = content_node.one('#statuses-wrapper');
543 select_all_handler = make_select_handler(node, statuses, true);536 selectors.all_link.on('click',
544 select_none_handler = make_select_handler(node, statuses, false);537 make_select_handler(node, statuses, true));
545 all_link.on('click', select_all_handler);538 selectors.none_link.on('click',
546 none_link.on('click', select_none_handler);539 make_select_handler(node, statuses, false));
547540
548 return accordion;541 return accordion;
549}542}
@@ -624,105 +617,125 @@
624}617}
625618
626/**619/**
620 * Add a recipient picker to the overlay.
621 */
622function add_recipient_picker(content_box, hide) {
623 var no_recipient_picker = Y.Node.create(
624 '<input type="hidden" name="recipient" value="user">' +
625 '<span>Yourself</span>');
626 var recipient_picker = Y.Node.create(
627 '<label><input type="radio" name="recipient" value="user" checked>' +
628 ' Yourself</label><br>' +
629 '<label><input type="radio" name="recipient" value="team">' +
630 ' One of the teams you administer</label><br>' +
631 '<dl style="margin-left:25px;">' +
632 ' <dt></dt>' +
633 ' <dd>' +
634 ' <select name="team" id="structural-subscription-teams">' +
635 ' </select>' +
636 ' </dd>' +
637 '</dl>');
638 var teams = LP.cache.administratedTeams;
639 var node = content_box.one('#bug-mail-recipient');
640 node.empty();
641 // Populate the team drop down from LP.cache data, if appropriate.
642 if (!hide && teams.length > 0) {
643 var select = recipient_picker.one('#structural-subscription-teams');
644 var i;
645 for (i=0; i<teams.length; i++) {
646 select.append(Y.Node.create('<option></option>')
647 .set('text', teams[i].title)
648 .set('value', teams[i].link));
649 }
650 select.on(
651 'focus',
652 function () {
653 Y.one('input[value="team"][name="recipient"]').set(
654 'checked', true);
655 }
656 );
657 node.append(recipient_picker);
658 } else {
659 node.append(no_recipient_picker);
660 }
661}
662
663/**
627 * Construct the overlay and populate it with the add/edit form.664 * Construct the overlay and populate it with the add/edit form.
628 */665 */
629function setup_overlay(content_box_id, hide_recipient_picker) {666function setup_overlay(content_box_id, hide_recipient_picker) {
630 var content_node = Y.one(content_box_id);667 var content_node = Y.one(content_box_id);
631 var container = Y.Node.create('<div id="overlay-container"></div>');668 var container = Y.Node.create(
632 var accordion_overlay_id = 'accordion-overlay';669 '<div id="overlay-container"><dl>' +
633 var teams = LP.cache.administratedTeams;670 ' <dt>Bug mail recipient</dt>' +
634 var no_recipient_picker =671 ' <dd id="bug-mail-recipient">' +
635 ' <input type="hidden" name="recipient" value="user">\n' +672 ' </dd>' +
636 ' <span>Yourself</span>\n',673 ' <dt>Subscription name</dt>' +
637 recipient_picker =674 ' <dd>' +
638 ' <input type="radio" name="recipient" value="user"\n'+675 ' <input type="text" name="name">' +
639 ' id="structural-subscription-recipient-user" checked>\n'+676 ' <a target="help" class="sprite maybe"' +
640 ' <label for="structural-subscription-recipient-user">\n'+677 ' href="/+help/structural-subscription-name.html">&nbsp;' +
641 ' Yourself</label><br>\n' +678 ' <span class="invisible-link">Structural subscription' +
642 ' <input type="radio" name="recipient"\n'+679 ' description help</span></a> ' +
643 ' id="structural-subscription-recipient-team"\n'+680 ' </dd>' +
644 ' value="team">\n'+681 ' <dt>Receive mail for bugs affecting' +
645 ' <label for="structural-subscription-recipient-team">One of\n'+682 ' <span id="structural-subscription-context-title"></span> that</dt>' +
646 ' the teams you administer</label><br>\n' +683 ' <dd>' +
647 ' <dl style="margin-left:25px;">\n' +684 ' <div id="events">' +
648 ' <dt></dt>\n' +685 ' <input type="radio" name="events"' +
649 ' <dd>\n' +686 ' value="added-or-closed"' +
650 ' <select name="team" id="structural-subscription-teams">\n'+687 ' id="added-or-closed" checked>' +
651 ' </select>\n' +688 ' <label for="added-or-closed">are added or ' +
652 ' </dd>\n' +689 ' closed</label>' +
653 ' </dl>\n',690 ' <br>' +
654 control_code =691 ' <input type="radio" name="events"' +
655 '<dl>\n' +692 ' value="added-or-changed"' +
656 ' <dt>Bug mail recipient</dt>\n' +693 ' id="added-or-changed">' +
657 ' <dd>\n' +694 ' <label for="added-or-changed">are added or changed in' +
658 ((!hide_recipient_picker && teams.length > 0) ?695 ' any way' +
659 recipient_picker : no_recipient_picker) +696 ' <em id="added-or-changed-more">(more options...)</em>' +
660 ' </dd>\n' +697 ' </label>' +
661 ' <dt>Subscription name</dt>\n' +698 ' </div>' +
662 ' <dd>\n' +699 ' <div id="filter-wrapper" class="ss-collapsible">' +
663 ' <input type="text" name="name">\n' +700 ' <dl style="margin-left:25px;">' +
664 ' <a target="help" class="sprite maybe"\n' +701 ' <dt></dt>' +
665 ' href="/+help/structural-subscription-name.html">&nbsp;\n' +702 ' <dd>' +
666 ' <span class="invisible-link">Structural subscription\n'+703 ' <input type="checkbox" name="filters"' +
667 ' description help</span></a>\n ' +704 ' value="filter-comments"' +
668 ' </dd>\n' +705 ' id="filter-comments">' +
669 ' <dt>Receive mail for bugs affecting\n'+706 ' <label for="filter-comments">Don\'t send mail about' +
670 ' <span id="structural-subscription-context-title">\n'+707 ' comments</label><br>' +
671 ' '+LP.cache.context.title+'</span> that</dt>\n' +708 ' <input type="checkbox" name="filters"' +
672 ' <dd>\n' +709 ' value="advanced-filter"' +
673 ' <div id="events">\n' +710 ' id="advanced-filter">' +
674 ' <input type="radio" name="events"\n' +711 ' <label for="advanced-filter">Bugs must match this' +
675 ' value="' + ADDED_OR_CLOSED + '"\n'+712 ' filter <em id="advanced-filter-more">(...)</em>' +
676 ' id="' + ADDED_OR_CLOSED + '" checked>\n'+713 ' </label><br>' +
677 ' <label for="'+ADDED_OR_CLOSED+'">are added or '+714 ' <div id="accordion-wrapper" ' +
678 ' closed</label>\n'+715 ' class="ss-collapsible">' +
679 ' <br>\n' +716 ' <dl>' +
680 ' <input type="radio" name="events"\n'+717 ' <dt></dt>' +
681 ' value="' + ADDED_OR_CHANGED + '"\n' +718 ' <dd style="margin-left:25px;">' +
682 ' id="' + ADDED_OR_CHANGED + '">\n'+719 ' <div id="accordion-overlay"' +
683 ' <label for="'+ADDED_OR_CHANGED+'">are added or changed in\n'+720 ' style="position:relative; overflow:hidden;"></div>' +
684 ' any way\n'+721 ' </dd>' +
685 ' <em id="'+ADDED_OR_CHANGED+'-more">(more options...)</em>\n'+722 ' </dl>' +
686 ' </label>\n' +723 ' </div> ' +
687 ' </div>\n' +724 ' </dd>' +
688 ' <div id="' + FILTER_WRAPPER + '" class="ss-collapsible">\n' +725 ' </dl>' +
689 ' <dl style="margin-left:25px;">\n' +726 ' </div> ' +
690 ' <dt></dt>\n' +727 ' </dd>' +
691 ' <dd>\n' +728 ' <dt></dt>' +
692 ' <input type="checkbox" name="filters"\n' +729 '</dl></div>');
693 ' value="' + FILTER_COMMENTS + '"\n'+730
694 ' id="'+FILTER_COMMENTS+'">\n' +731 // Assemble some nodes and set the title.
695 ' <label for="'+FILTER_COMMENTS+'">Don\'t send mail about\n'+732 content_node
696 ' comments</label><br>\n' +733 .appendChild(container)
697 ' <input type="checkbox" name="filters"\n' +734 .one('#structural-subscription-context-title')
698 ' value="' + ADVANCED_FILTER + '"\n' +735 .set('text', LP.cache.context.title);
699 ' id="' + ADVANCED_FILTER + '">\n' +736
700 ' <label for="'+ADVANCED_FILTER+'">Bugs must match this\n'+737 var accordion = create_accordion('#accordion-overlay', content_node);
701 ' filter <em id="'+ADVANCED_FILTER+'-more">(...)</em>\n'+738 add_recipient_picker(container, hide_recipient_picker);
702 ' </label><br>\n' +
703 ' <div id="' + ACCORDION_WRAPPER + '" \n' +
704 ' class="' + SS_COLLAPSIBLE + '">\n' +
705 ' <dl>\n' +
706 ' <dt></dt>\n' +
707 ' <dd style="margin-left:25px;">\n' +
708 ' <div id="' + accordion_overlay_id + '"\n' +
709 ' style="position:relative; '+
710 'overflow:hidden;"></div>\n' +
711 ' </dd>\n' +
712 ' </dl>\n' +
713 ' </div> \n' +
714 ' </dd>\n' +
715 ' </dl>\n' +
716 ' </div> \n' +
717 ' </dd>\n' +
718 ' <dt></dt>\n' +
719 '</dl>';
720
721 content_node.appendChild(container);
722 container.appendChild(Y.Node.create(control_code));
723
724 var accordion = create_accordion(
725 '#' + accordion_overlay_id, content_node);
726739
727 // Set up click handlers for the events radio buttons.740 // Set up click handlers for the events radio buttons.
728 var radio_group = Y.all('#events input');741 var radio_group = Y.all('#events input');
@@ -735,26 +748,6 @@
735 advanced_filter.on(748 advanced_filter.on(
736 'change',749 'change',
737 function() {handle_change(ADVANCED_FILTER, ACCORDION_WRAPPER);});750 function() {handle_change(ADVANCED_FILTER, ACCORDION_WRAPPER);});
738 // Populate the team drop down from LP.cache data, if appropriate.
739 if (!hide_recipient_picker && teams.length > 0) {
740 var select = Y.one('#structural-subscription-teams');
741 var i;
742 var team;
743 for (i=0; i<teams.length; i++) {
744 team = teams[i];
745 var option = Y.Node.create('<option></option>');
746 option.set(INNER_HTML, team.title);
747 option.set(VALUE, team.link);
748 select.appendChild(option);
749 }
750 select.on(
751 'focus',
752 function () {
753 Y.one('input[value="team"][name="recipient"]').set(
754 'checked', true);
755 }
756 );
757 }
758 return '#' + container._node.id;751 return '#' + container._node.id;
759} // setup_overlay752} // setup_overlay
760// Expose in the namespace for testing purposes.753// Expose in the namespace for testing purposes.
@@ -807,7 +800,7 @@
807 var overlay_id = setup_overlay(config.content_box, true);800 var overlay_id = setup_overlay(config.content_box, true);
808 var submit_button = Y.Node.create(801 var submit_button = Y.Node.create(
809 '<button type="submit" name="field.actions.create" ' +802 '<button type="submit" name="field.actions.create" ' +
810 'value="Save Changes" class="lazr-pos lazr-btn" '+803 'value="Save Changes" class="lazr-pos lazr-btn" ' +
811 '>OK</button>');804 '>OK</button>');
812 // This is a bit of an odd approach, but it lets us retrofit code805 // This is a bit of an odd approach, but it lets us retrofit code
813 // without a large refactoring. When edit_subscription_handler is806 // without a large refactoring. When edit_subscription_handler is
@@ -848,8 +841,9 @@
848 overlay.field_errors[field_name] = true;841 overlay.field_errors[field_name] = true;
849 // Accordion sets fixed height for the accordion item,842 // Accordion sets fixed height for the accordion item,
850 // so we have to resize the tags container.843 // so we have to resize the tags container.
851 if (field_name == 'tags')844 if (field_name === 'tags') {
852 tags_container.resize();845 tags_container.resize();
846 }
853 // Firefox prohibits focus from inside the 'focus lost' event847 // Firefox prohibits focus from inside the 'focus lost' event
854 // handler (probably to stop loops), so we need to run848 // handler (probably to stop loops), so we need to run
855 // it from a different context (which we do with setTimeout).849 // it from a different context (which we do with setTimeout).
@@ -859,7 +853,7 @@
859 overlay.field_errors[field_name] = false;853 overlay.field_errors[field_name] = false;
860 }854 }
861 });855 });
862};856}
863857
864function get_error_for_tags_list(value) {858function get_error_for_tags_list(value) {
865 // See database/schema/trusted.sql valid_name() function859 // See database/schema/trusted.sql valid_name() function
@@ -874,7 +868,8 @@
874 'digits 0-9 and symbols "+", "-" or ".", and they ' +868 'digits 0-9 and symbols "+", "-" or ".", and they ' +
875 'must start with a lowercase letter or a digit.');869 'must start with a lowercase letter or a digit.');
876 }870 }
877};871}
872
878// Export for testing873// Export for testing
879namespace._get_error_for_tags_list = get_error_for_tags_list;874namespace._get_error_for_tags_list = get_error_for_tags_list;
880875
@@ -956,12 +951,12 @@
956 var i;951 var i;
957 for (i=0; i<teams.length; i++) {952 for (i=0; i<teams.length; i++) {
958 if (teams[i].link === filter_info.subscriber_link){953 if (teams[i].link === filter_info.subscriber_link){
959 recipient_label.set(INNER_HTML, teams[i].title);954 recipient_label.set('text', teams[i].title);
960 break;955 break;
961 }956 }
962 }957 }
963 } else {958 } else {
964 recipient_label.set(INNER_HTML, 'Yourself');959 recipient_label.set('text', 'Yourself');
965 }960 }
966 content_node.one('[name="name"]').set('value',filter.description);961 content_node.one('[name="name"]').set('value',filter.description);
967 if (is_lifecycle) {962 if (is_lifecycle) {
@@ -1011,10 +1006,10 @@
1011 context.filter_info = filter_info;1006 context.filter_info = filter_info;
1012 context.filter_id = filter_id;1007 context.filter_id = filter_id;
1013 var title = subscription.target_title;1008 var title = subscription.target_title;
1014 Y.one('#structural-subscription-context-title').set(1009 Y.one('#structural-subscription-context-title')
1015 INNER_HTML, title);1010 .set('text', title);
1016 Y.one('#subscription-overlay-title').set(1011 Y.one('#subscription-overlay-title')
1017 INNER_HTML, 'Edit subscription for '+title+' bugs');1012 .set('text', 'Edit subscription for '+title+' bugs');
1018 add_subscription_overlay.show();1013 add_subscription_overlay.show();
1019 }1014 }
1020 };1015 };
@@ -1035,14 +1030,14 @@
1035 headers: {'X-HTTP-Method-Override': 'DELETE'},1030 headers: {'X-HTTP-Method-Override': 'DELETE'},
1036 on: {success: function(transactionid, response, args){1031 on: {success: function(transactionid, response, args){
1037 var subscriber = Y.one(1032 var subscriber = Y.one(
1038 '#subscription-'+subscriber_id.toString()),1033 '#subscription-'+subscriber_id.toString());
1039 node = subscriber,1034 var to_collapse = subscriber;
1040 filters = subscriber.all('.subscription-filter');1035 var filters = subscriber.all('.subscription-filter');
1041 if (!filters.isEmpty()) {1036 if (!filters.isEmpty()) {
1042 node = Y.one(1037 to_collapse = Y.one(
1043 '#subscription-filter-'+filter_id.toString());1038 '#subscription-filter-'+filter_id.toString());
1044 }1039 }
1045 collapse_node(node);1040 collapse_node(to_collapse);
1046 },1041 },
1047 failure: error_handler.getFailureHandler()1042 failure: error_handler.getFailureHandler()
1048 }1043 }
@@ -1066,11 +1061,12 @@
1066 var filter_info = sub.filters[j];1061 var filter_info = sub.filters[j];
1067 if (!filter_info.subscriber_is_team ||1062 if (!filter_info.subscriber_is_team ||
1068 filter_info.user_is_team_admin) {1063 filter_info.user_is_team_admin) {
1069 var edit_link = Y.one('#edit-'+filter_id.toString());1064 var node = Y.one('#subscription-filter-'+filter_id.toString());
1065 var edit_link = node.one('a.edit-subscription');
1070 var edit_handler = make_edit_handler(1066 var edit_handler = make_edit_handler(
1071 sub, filter_info, filter_id, config, context);1067 sub, filter_info, filter_id, config, context);
1072 edit_link.on('click', edit_handler);1068 edit_link.on('click', edit_handler);
1073 var delete_link = Y.one('#unsubscribe-'+filter_id.toString());1069 var delete_link = node.one('a.delete-subscription');
1074 var delete_handler = make_delete_handler(1070 var delete_handler = make_delete_handler(
1075 filter_info.filter, filter_id, i);1071 filter_info.filter, filter_id, i);
1076 delete_link.on('click', delete_handler);1072 delete_link.on('click', delete_handler);
@@ -1085,22 +1081,27 @@
1085 */1081 */
1086function fill_in_bug_subscriptions(config, context) {1082function fill_in_bug_subscriptions(config, context) {
1087 validate_config(config);1083 validate_config(config);
1084
1088 var listing = Y.one('#subscription-listing');1085 var listing = Y.one('#subscription-listing');
1089 var subscription_info = LP.cache.subscription_info;1086 var subscription_info = LP.cache.subscription_info;
1090 var html = '<div class="yui-g"><div id="structural-subscriptions">';1087 var top_node = Y.Node.create(
1088 '<div class="yui-g"><div id="structural-subscriptions"></div></div>');
1091 var filter_id = 0;1089 var filter_id = 0;
1092 var i;1090 var i;
1093 var j;1091 var j;
1094 for (i=0; i<subscription_info.length; i++) {1092 for (i=0; i<subscription_info.length; i++) {
1095 var sub = subscription_info[i];1093 var sub = subscription_info[i];
1096 html +=1094 var sub_node = top_node.appendChild(Y.Node.create(
1097 '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+1095 '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+
1098 ' border: 1px solid #ddd;"'+1096 ' border: 1px solid #ddd;"></div>')
1099 ' id="subscription-'+i.toString()+'">'+1097 .set('id', 'subscription-'+i.toString()));
1100 ' <span style="float: left; margin-top: -0.6em; padding: 0 1ex;'+1098 sub_node.appendChild(Y.Node.create(
1101 ' background-color: #fff;">Subscriptions to'+1099 ' <span style="float: left; margin-top: -0.6em; '+
1102 ' <a href="'+sub.target_url+'">'+sub.target_title+'</a>'+1100 ' padding: 0 1ex; background-color: #fff;"></a>'))
1103 ' </span>';1101 .appendChild('<span>Subscriptions to </span>')
1102 .appendChild(Y.Node.create('<a></a>')
1103 .set('href', sub.target_url)
1104 .set('text', sub.target_title));
11041105
1105 for (j=0; j<sub.filters.length; j++) {1106 for (j=0; j<sub.filters.length; j++) {
1106 var filter = sub.filters[j].filter;1107 var filter = sub.filters[j].filter;
@@ -1110,52 +1111,53 @@
1110 // and see the information you expect to see.1111 // and see the information you expect to see.
1111 LP.cache['structural-subscription-filter-'+filter_id.toString()] =1112 LP.cache['structural-subscription-filter-'+filter_id.toString()] =
1112 filter;1113 filter;
1113 html +=1114 var filter_node = sub_node.appendChild(Y.Node.create(
1114 '<div style="margin: 1em 0em 0em 1em"'+1115 '<div style="margin: 1em 0em 0em 1em"'+
1115 ' id="subscription-filter-'+filter_id.toString()+'"'+1116 ' class="subscription-filter"></div>')
1116 ' class="subscription-filter">'+1117 .set('id', 'subscription-filter-'+filter_id.toString()))
1117 ' <div style="margin-top: 1em">'+1118 .appendChild(Y.Node.create(
1118 ' <strong id="filter-name-'+1119 '<div style="margin-top: 1em"></div>'));
1119 filter_id.toString()+'">'+1120 filter_node.appendChild(Y.Node.create(
1120 render_filter_name(sub.filters[j], filter)+'</strong>';1121 '<strong class="filter-name"></strong>'))
1122 .appendChild(render_filter_title(sub.filters[j], filter));
1123
1121 if (!sub.filters[j].subscriber_is_team ||1124 if (!sub.filters[j].subscriber_is_team ||
1122 sub.filters[j].user_is_team_admin) {1125 sub.filters[j].user_is_team_admin) {
1123 // User can edit the subscription.1126 // User can edit the subscription.
1124 html +=1127 filter_node.appendChild(Y.Node.create(
1125 '<span style="float: right">'+1128 '<span style="float: right">'+
1126 '<a href="#" class="sprite modify edit js-action"'+1129 '<a href="#" class="sprite modify edit js-action '+
1127 ' id="edit-'+filter_id.toString()+'">'+1130 ' edit-subscription">'+
1128 ' Edit this subscription</a> or '+1131 ' Edit this subscription</a> or '+
1129 '<a href="#" class="sprite modify remove js-action"'+1132 '<a href="#" class="sprite modify remove js-action '+
1130 ' id="unsubscribe-'+filter_id.toString()+'">'+1133 ' delete-subscription">'+
1131 ' Unsubscribe</a></span>';1134 ' Unsubscribe</a></span>'));
1132 } else {1135 } else {
1133 // User cannot edit the subscription, because this is a1136 // User cannot edit the subscription, because this is a
1134 // team and the user does not have admin privileges.1137 // team and the user does not have admin privileges.
1135 html +=1138 filter_node.appendChild(Y.Node.create(
1136 '<span style="float: right"><em>'+1139 '<span style="float: right"><em>'+
1137 'You do not have privileges to change this subscription'+1140 'You do not have privileges to change this subscription'+
1138 '</em></span>';1141 '</em></span>'));
1139 }1142 }
1140 html += '</div>';1143
1141 html +=1144 filter_node.appendChild(Y.Node.create(
1142 '<div style="padding-left: 1em"'+1145 '<div style="padding-left: 1em"></div>')
1143 ' id="filter-description-'+filter_id.toString()+'">'+1146 .set('id', 'filter-description-'+filter_id.toString()))
1144 render_filter_description(filter)+'</div>';1147 .appendChild(create_filter_description(filter));
11451148
1146 html += '</div>';
1147 filter_id += 1;1149 filter_id += 1;
1148 }1150 }
11491151
1150 // We can remove this once we enforce at least one filter per1152 // We can remove this once we enforce at least one filter per
1151 // subscription.1153 // subscription.
1152 if (subscription_info[i].filters.length === 0) {1154 if (subscription_info[i].filters.length === 0) {
1153 html += '<strong>All messages</strong>';1155 sub_node.appendChild(
1156 '<div style="clear: both; padding: 1em 0 0 1em"></div>')
1157 .appendChild('<strong>All messages</strong>');
1154 }1158 }
1155 html += '</div>';
1156 }1159 }
1157 html += '</div></div>';1160 listing.appendChild(top_node);
1158 listing.appendChild(Y.Node.create(html));
11591161
1160 wire_up_edit_links(config, context);1162 wire_up_edit_links(config, context);
1161}1163}
@@ -1163,7 +1165,8 @@
1163/**1165/**
1164 * Construct a one-line textual description of a filter's name.1166 * Construct a one-line textual description of a filter's name.
1165 */1167 */
1166function render_filter_name(filter_info, filter) {1168function render_filter_title(filter_info, filter) {
1169 var title = Y.Node.create('<span></span>');
1167 var description;1170 var description;
1168 if (filter.description) {1171 if (filter.description) {
1169 description = '"'+filter.description+'"';1172 description = '"'+filter.description+'"';
@@ -1171,62 +1174,79 @@
1171 description = '(unnamed)';1174 description = '(unnamed)';
1172 }1175 }
1173 if (filter_info.subscriber_is_team) {1176 if (filter_info.subscriber_is_team) {
1174 return '<a href="'+filter_info.subscriber_url+'">'+1177 title.appendChild(Y.Node.create('<a></a>'))
1175 filter_info.subscriber_title+"</a> subscription: "+description;1178 .set('href', filter_info.subscriber_url)
1179 .set('text', filter_info.subscriber_title);
1180 title.appendChild(Y.Node.create('<span></span>'))
1181 .set('text', ' subscription: '+description);
1176 } else {1182 } else {
1177 return 'Your subscription: '+description;1183 title.set('text', 'Your subscription: '+description);
1178 }1184 }
1185 return title;
1179}1186}
11801187
1181/**1188/**
1182 * Construct a textual description of all of filter's properties.1189 * Construct a textual description of all of filter's properties.
1183 */1190 */
1184function render_filter_description(filter) {1191function create_filter_description(filter) {
1185 var html = '';1192 var description = Y.Node.create('<div></div>');
1186 var filter_items = '';1193
1194 var filter_items = [];
1187 // Format status conditions.1195 // Format status conditions.
1188 if (filter.statuses.length !== 0) {1196 if (filter.statuses.length !== 0) {
1189 filter_items += '<li> have status ' +1197 filter_items.push(Y.Node.create('<li></li>')
1190 filter.statuses.join(', ');1198 .set('text', 'have status '+filter.statuses.join(', ')));
1191 }1199 }
11921200
1193 // Format importance conditions.1201 // Format importance conditions.
1194 if (filter.importances.length !== 0) {1202 if (filter.importances.length !== 0) {
1195 filter_items += '<li> are of importance ' +1203 filter_items.push(Y.Node.create('<li></li>')
1196 filter.importances.join(', ');1204 .set('text', 'are of importance '+filter.importances.join(', ')));
1197 }1205 }
11981206
1199 // Format tag conditions.1207 // Format tag conditions.
1200 if (filter.tags.length !== 0) {1208 if (filter.tags.length !== 0) {
1201 filter_items += '<li> are tagged with ';1209 var tag_desc = Y.Node.create('<li>are tagged with </li>')
1210 .append(Y.Node.create('<strong></strong>'))
1211 .append(Y.Node.create('<span> of these tags: </span>'))
1212 .append(Y.Node.create('<span></span>')
1213 .set('text', filter.tags.join(', ')));
1214
1202 if (filter.find_all_tags) {1215 if (filter.find_all_tags) {
1203 filter_items += '<strong>all</strong>';1216 tag_desc.one('strong').set('text', 'all');
1204 } else {1217 } else {
1205 filter_items += '<strong>any</strong>';1218 tag_desc.one('strong').set('text', 'any');
1206 }1219 }
1207 filter_items += ' of these tags: ' +1220 filter_items.push(tag_desc);
1208 filter.tags.join(', ');
1209 }1221 }
12101222
1211 // If there were any conditions to list, stich them in with an1223 // If there were any conditions to list, stich them in with an
1212 // intro.1224 // intro.
1213 if (filter_items !== '') {1225 if (filter_items.length > 0) {
1214 html += 'You are subscribed to bugs that'+1226 var ul = Y.Node.create('<ul class="bulleted"></ul>');
1215 '<ul class="bulleted">'+filter_items+'</ul>';1227 Y.each(filter_items, function (li) {ul.appendChild(li);});
1228 description.appendChild(
1229 Y.Node.create('<span>You are subscribed to bugs that</span>'));
1230 description.appendChild(ul);
1216 }1231 }
12171232
1218 // Format event details.1233 // Format event details.
1234 var events; // When will email be sent?
1219 if (filter.bug_notification_level === 'Discussion') {1235 if (filter.bug_notification_level === 'Discussion') {
1220 html += 'You will recieve an email when any change '+1236 events = 'You will recieve an email when any change '+
1221 'is made or a comment is added.';1237 'is made or a comment is added.';
1222 } else if (filter.bug_notification_level === 'Details') {1238 } else if (filter.bug_notification_level === 'Details') {
1223 html += 'You will recieve an email when any changes '+1239 events = 'You will recieve an email when any changes '+
1224 'are made to the bug. Bug comments will not be sent.';1240 'are made to the bug. Bug comments will not be sent.';
1225 } else if (filter.bug_notification_level === 'Lifecycle') {1241 } else if (filter.bug_notification_level === 'Lifecycle') {
1226 html += 'You will recieve an email when bugs are '+1242 events = 'You will recieve an email when bugs are '+
1227 'opened or closed.';1243 'opened or closed.';
1244 } else {
1245 throw new Error('Unrecognized events.');
1228 }1246 }
1229 return html;1247 description.appendChild(Y.Node.create(events));
1248
1249 return description;
1230}1250}
12311251
1232/**1252/**
12331253
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-03-30 15:23:42 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-01 17:57:54 +0000
@@ -147,11 +147,12 @@
147 test_make_selector_controls: function() {147 test_make_selector_controls: function() {
148 // Verify the creation of select all/none controls.148 // Verify the creation of select all/none controls.
149 var selectors = module.make_selector_controls('sharona');149 var selectors = module.make_selector_controls('sharona');
150 Assert.areEqual('sharona-select-all', selectors['all_name']);150 Assert.areEqual(
151 Assert.areEqual('sharona-select-none', selectors['none_name']);151 'Select all', selectors.all_link.get('text'));
152 Assert.areEqual(152 Assert.areEqual(
153 '<div id="sharona-selectors"',153 'Select none', selectors.none_link.get('text'));
154 selectors['html'].slice(0, 27));154 Assert.areEqual(
155 'sharona-selectors', selectors.node.get('id'));
155 }156 }
156 });157 });
157 suite.add(test_case);158 suite.add(test_case);
@@ -419,8 +420,8 @@
419 // accordion pane.420 // accordion pane.
420 module.setup(this.configuration);421 module.setup(this.configuration);
421 var checkboxes = Y.all('input[name="importances"]');422 var checkboxes = Y.all('input[name="importances"]');
422 var select_all = Y.one('#importances-select-all');423 var select_all = Y.one('#importances-selectors > a.select-all');
423 var select_none = Y.one('#importances-select-none');424 var select_none = Y.one('#importances-selectors > a.select-none');
424 Assert.isTrue(test_checked(checkboxes, true));425 Assert.isTrue(test_checked(checkboxes, true));
425 // Simulate a click on the select_none control.426 // Simulate a click on the select_none control.
426 Y.Event.simulate(Y.Node.getDOMNode(select_none), 'click');427 Y.Event.simulate(Y.Node.getDOMNode(select_none), 'click');
@@ -435,8 +436,8 @@
435 // accordion pane.436 // accordion pane.
436 module.setup(this.configuration);437 module.setup(this.configuration);
437 var checkboxes = Y.all('input[name="statuses"]');438 var checkboxes = Y.all('input[name="statuses"]');
438 var select_all = Y.one('#statuses-select-all');439 var select_all = Y.one('#statuses-selectors > a.select-all');
439 var select_none = Y.one('#statuses-select-none');440 var select_none = Y.one('#statuses-selectors > a.select-none');
440 Assert.isTrue(test_checked(checkboxes, true));441 Assert.isTrue(test_checked(checkboxes, true));
441 // Simulate a click on the select_none control.442 // Simulate a click on the select_none control.
442 Y.Event.simulate(Y.Node.getDOMNode(select_none), 'click');443 Y.Event.simulate(Y.Node.getDOMNode(select_none), 'click');