TPAC: Don't load dojo widgets unless we actually need them (for autocomplete)

Bug #1411699 reported by Dan Scott on 2015-01-16
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

* Evergreen master

Currently, a number of conditions result in all of dojo -- including the widgets -- getting loaded. Although the JavaScript is cached for subsequent requests, this has a heavy first-load cost which can affect the user experience negatively. If we can make the widgets only be loaded when absolutely necessary (that is, when autocomplete is enabled), then we can make the page faster to load and improve the user experience.

Dan Scott (denials) on 2015-01-16
Changed in evergreen:
milestone: none → 2.8-beta
Ben Shum (bshum) on 2015-04-02
Changed in evergreen:
milestone: 2.8-beta →
status: New → Confirmed
importance: Undecided → Wishlist
Dan Scott (denials) wrote :

Dojo was turned on unconditionally due to 92e84a389e because Dojo is currently required for the copy locations advanced search filter, a core feature, as part of bug #1314370. Loading Dojo adds roughly 600ms to page load and rendering time for functionality that is only necessary on a single page.

While the copy locations search filter is a core feature, we can still make it conditional so that Dojo is only loaded on the advanced search page using something like:

IF == 'advanced';
    want_dojo = 1;

This would optimize the performance for every page other than the advanced search page.

Dan Scott (denials) on 2017-04-14
tags: added: performance
Dan Scott (denials) wrote :

A more ambitious branch that removes the need for Dojo entirely from copy location search is at;a=shortlog;h=refs/heads/user/dbs/lp1411699_dojoless_adv_copyloc

The caveat of this branch is that it currently requires Template::Plugin::CGI and OpenILS::WWW::EGCatLoader to be tweaked to use the "-oldstyle_urls" pragma (for ampersands instead of semicolon delimiters between query params). See also #1687545.

Galen Charlton (gmc) wrote :

I've pushed a signoff as well as a follow-up to user/gmcharlt/lp1411699_signoff:;a=shortlog;h=refs/heads/user/gmcharlt/lp1411699_signoff

The follow-up is pretty much just an abundance of caution thing to allow the main patch to go in sooner rather than later; I expect that its need will be obviated well before the release of 3.0.

tags: added: signedoff
Changed in evergreen:
milestone: → 3.0-alpha
Galen Charlton (gmc) wrote :

Also noting that IMO separate release notes aren't needed for this one: we can just have one for the overall project of removing Dojo from the public catalog.

Galen Charlton (gmc) wrote :

Noting per IRC conversation yesterday the existence of a WIP branch by Dan that removes use of Dojo from acjs:;a=shortlog;h=refs/heads/user/dbs/nodojo_acjs

Bill Erickson (berick) wrote :

Just to clarify, we want to merge user/gmcharlt/lp1411699_signoff for this bug and the other branches noted here will be part of new/other bugs?

Galen Charlton (gmc) wrote :

Bill, I think so. Please go ahead and give user/gmcharlt/lp1411699_signoff a final check and merge it.

Bill Erickson (berick) on 2017-08-03
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Bill Erickson (berick) wrote :

Confirmed dojo loads (or not) as expected. Thanks Dan and Galen. Merged to master.

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Bill Erickson (berick) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers