Ajax controls disabled on bugs with many tasks due to multiple redundant copies of person picker make_picker function in view-source:https://bugs.launchpad.net/launchpad-project/+bugs?advanced=1

Bug #707234 reported by Robert Collins
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Critical
j.c.sackett

Bug Description

We have multiple person picker instances on the page, and in the source the script segment below is emitted once for each, which is wasteful in a few ways:
- the page is larger
- more to evaluate

<script>
LPS.use('node', 'lp.app.picker', 'plugin', function(Y) {
    if (Y.UA.ie) {
        return;
    }

    // Args from python.
    var args = {"header": "Select a Person or Team", "show_widget_id": "show-widget-field-subscriber", "vocabulary": "ValidPersonOrTeam", "input_id": "field.subscriber", "step_title": "Search", "extra_no_results_message": null};

    // The vocabulary picker, created when used for the first time.
    var picker = null;
    function make_picker() {
        var config = {
            header: args.header,
            step_title: args.step_title,
            extra_no_results_message: args.extra_no_results_message
        };
        var picker = Y.lp.app.picker.create(args.vocabulary, config);
        if (config.extra_no_results_message !== null) {
            picker.before('resultsChange', function (e) {
                var new_results = e.details[0].newVal;
                if (new_results.length === 0) {
                    picker.set('footer_slot',
                               Y.Node.create(config.extra_no_results_message));
                }
                else {
                    picker.set('footer_slot', null);
                }
            });
        }
        picker.plug(Y.lazr.TextFieldPickerPlugin,
                    {input_element: '[id="' + args.input_id + '"]'});
        return picker;
    }

    // Sort out the "Choose..." link.
    var show_widget_node = Y.get('#' + args.show_widget_id);
    show_widget_node.set('innerHTML', 'Choose&hellip;');
    show_widget_node.addClass('js-action');
    show_widget_node.get('parentNode').removeClass('unseen');
    show_widget_node.on('click', function (e) {
        if (picker === null) {
            picker = make_picker();
        }
        picker.show();
        e.preventDefault();
    });
});

</script>

Related branches

summary: - multiple redundant copies of person picker make_picker function in view-
+ Ajax controls disabled on bugs with many tasks due to multiple redundant
+ copies of person picker make_picker function in view-
source:https://bugs.launchpad.net/launchpad-project/+bugs?advanced=1
Curtis Hovey (sinzui)
tags: added: person-picker regression
tags: added: disclosure
Curtis Hovey (sinzui)
Changed in launchpad:
importance: High → Critical
j.c.sackett (jcsackett)
Changed in launchpad:
assignee: nobody → j.c.sackett (jcsackett)
status: Triaged → In Progress
Revision history for this message
Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in launchpad:
status: In Progress → Fix Committed
Revision history for this message
j.c.sackett (jcsackett) wrote :

The branch that landed removed the redundant javascript emissions, but the picker links are still a no-show on the advanced bug search page.

tags: added: qa-ok
removed: qa-needstesting
Changed in launchpad:
status: Fix Committed → In Progress
Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks like on further analysis the fix broke some other pickers that weren't tested. I'm preparing a rollback.

tags: added: qa-bad
removed: qa-ok
Ursula Junque (ursinha)
tags: added: bad-commit-13251
j.c.sackett (jcsackett)
tags: removed: qa-bad
Revision history for this message
Francis J. Lacoste (flacoste) wrote :

The qa-ok tag is there just to make qatagger consider the bad revision has not a blocker (it was rolledback in 13260, but with a wrong rollback= revision number).

tags: added: qa-ok
Revision history for this message
Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
removed: qa-ok
Changed in launchpad:
status: In Progress → Fix Committed
Revision history for this message
j.c.sackett (jcsackett) wrote :

The actual error was caused by some code trying to setup structural subscriptions on a link that doesn't exist on the advanced page; the fix deals with that, not the multiple emitted javascript.

Bug 799847 is about bad javascript creation in form-macros, and it has been updated to include mention of the redundant js.

tags: added: qa-ok
removed: qa-needstesting
William Grant (wgrant)
Changed in launchpad:
status: Fix Committed → Fix Released
status: Fix Released → In Progress
Revision history for this message
j.c.sackett (jcsackett) wrote :

Moving this back to fixed release; the remaining unmerged branch was erroneously linked.

Changed in launchpad:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.