Launchpad itself

Merge lp://qastaging/~gmb/launchpad/bug-495029 into lp://qastaging/launchpad

  1. bug-495029
  2. Merge into devel
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://qastaging/~gmb/launchpad/bug-495029
Merge into: lp://qastaging/launchpad
Diff against target: 38 lines (+6/-8)
1 file modified
lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js (+6/-8)
To merge this branch: bzr merge lp://qastaging/~gmb/launchpad/bug-495029
Reviewer Review Type Date Requested Status
Deryck Hodge (community) code Approve
Review via email: mp+15944@code.qastaging.launchpad.net

Commit message

The inline dupefinder will now show a bug reporting form if no similar bugs are found.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes bug 495029 by converting the Y.Lang.isValue() checks on the results of Y.all() to use result.size().

The problem is due in part the upgrade in YUI. The dupefinder was written and mostly QA'd against a branch which used the pre-release YUI3, which expected Y.all() to return null when no matching nodes were found. The released version of YUI3's Y.all() returns an empty NodeList when no nodes are found. Since the change for Y.Lang.isValue() on such results would now always return true, the JS assumed that there were duplicates found when there weren't and so would never show the filebug form in those cases.

Revision history for this message
Deryck Hodge (deryck) wrote :

Looks good.

Cheers,
deryck

review: Approve (js)
Revision history for this message
Deryck Hodge (deryck) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js'
2--- lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js 2009-12-09 13:20:27 +0000
3+++ lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js 2009-12-10 13:49:19 +0000
4@@ -155,7 +155,7 @@
5
6 bug_already_reported_expanders = Y.all(
7 'img.bug-already-reported-expander');
8- if (Y.Lang.isValue(bug_already_reported_expanders)) {
9+ if (bug_already_reported_expanders.size() > 0) {
10 // If there are duplicates shown, set up the JavaScript of
11 // the duplicates that have been returned.
12 Y.bugs.setup_dupes();
13@@ -255,12 +255,10 @@
14 // Add an on-click handler to the radio buttons to ensure that their
15 // labels' styles are set correctly when they're selected.
16 var radio_buttons = form.queryAll('input.subscribe-option');
17- if (Y.Lang.isValue(radio_buttons)) {
18- Y.each(radio_buttons, function(radio_button) {
19- var weight = radio_button.get('checked') ? 'bold' : 'normal';
20- radio_button.get('parentNode').setStyle('fontWeight', weight);
21- });
22- }
23+ Y.each(radio_buttons, function(radio_button) {
24+ var weight = radio_button.get('checked') ? 'bold' : 'normal';
25+ radio_button.get('parentNode').setStyle('fontWeight', weight);
26+ });
27
28 return subscribe_form_overlay;
29 }
30@@ -308,7 +306,7 @@
31 'img.bug-already-reported-expander');
32 bug_reporting_form = Y.one('#bug_reporting_form');
33
34- if (Y.Lang.isValue(bug_already_reported_expanders)) {
35+ if (bug_already_reported_expanders.size() > 0) {
36 // Collapse all the details divs, since we don't want them
37 // expanded first up.
38 Y.each(Y.all('div.duplicate-details'), function(div) {