web client: "Set default view" breaks record page loading

[Chris Sharp]

While trying to reproduce the problem in bug 1724348, I found that if I click "Set default view" for the MARC View tab (or any other tab), loading a record page directly (e.g. https://myserver.com/eg/staff/cat/catalog/record/124) initially loads the preferred view, then "resets" itself, first loading the splash screen, then loading an OPAC view with no data. If I manually remove the eg.cat.default_record_tab setting, then reload, the record page loads correctly in OPAC view.

Evergreen 3.0 through current master
OpenSRF 3.0.0/master
PostgreSQL 9.5
Ubuntu 16.04 LTS

[Kathy Lussier (klussier)]

I'm not able to replicate this one on a master system. It sounds very similar to bug 1708951, which was fixed before the 3.0.0 release.

[Andrea Neiman (aneiman)]

I wasn't able to replicate this on webby 3.0.0 with a direct link, however I think there may be a regression of the issues in bug 1708951 which I documented separately at bug 1731305.

[Anna Goben (agoben)]

I can easily replicate this on our production system (3.0.?).

[Anna Goben (agoben)]

I also see this behavior actively this morning on the Webby demo server.

[Chris Sharp (chrissharp123)]

I came here to describe the steps to reproduce this without realizing that Anna had already posted this helpful video. Steps in text form:

1) search on click on a record

2) click on one of the other tabs and click "Set default view"

3) reload the page.

To get it working again, click on OPAC View and click "Set default view", then reload.

[Kathy Lussier (klussier)]

It could just be coincidence, but once I upgraded my Chrome browser from version 60 to 62, I started seeing this problem.

[Chris Sharp (chrissharp123)]

I should have mentioned the browser from the beginning. Yes, this is happening in Chrome 62. Firefox 57 appears to handle things correctly. I'm using both on Fedora 27.

[Lynn Floyd (lfloyd)]

Just thought you would like to know, tried it with Chrome 62 on a Windows 7 and it works like expected. Have not tried it with Firefox yet.

[Chris Sharp (chrissharp123)]

This is consistently happening for me on Chrome 62. Attaching verbose console log with a notation for when the page reloads.

[Kathy Lussier (klussier)]

I notice that his also occurs if you set the holdings view as the default view.

[John Yorio (jyorio)]

Just noting that the "Show in Catalog" option from z39.50 is another way to encounter this issue.

[a. bellenir (abellenir)]

something sets the iframe url to $BASE_URL/{{url}} (encoded as %7B%7Burl%7D%7D) after a reload. this url isn't valid and the page ultimately fails to load, as described above.

there is now a branch with a quick fix for this: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/abellenir/lp1731272-record-reload

it to avoids firing the $scope.onchange event with an invalid url, but we may want to consider finding and correcting what sets the {{url}} in the first place instead.

some related problems remain. i'm not sure if it would be better to create new bugs for them or to keep everything together here, so please let me know:

* the query is present in the page URL (and in $routeParams), but no other query-related parameters (eg: qtype) are remembered. search results are lost when the page is reloaded. the "Back To Results" button says "(1/1)", and is disabled.

* the record_tab is not preserved in the page url, so the default tab will be shown on reload instead of the original tab. the desired tab will be shown if it is specified via querystring (eg: ...&record_tab=marc_html).

* after a reload the opac tab will briefly show the splash screen before loading search results (if on the results list page) or the desired record (if viewing a particular record).

[Jeff Davis (jdavis-sitka)]

The iframe {{url}} issue is also relevant to bug 1761276.

[Jennifer Pringle (jpringle-u)]

I tested this fix and it works almost perfectly for me. The only issue I've encountered is that if View Holds is set as my default view the page usually loads with progress bar that is stuck at 0%. If I click anywhere on the page the progress bar disappears and any holds display. Occasionally View Holds will load, briefly show the progress bar, and then display the existing holds.

[a. bellenir (abellenir)]

the "hanging progress bar" appears for records that do not have any holds.

[a. bellenir (abellenir)]

this stems from something goofy with timing and promises in angular. i didn't manage to track down a good fix, but someone more knowledgeable about the inner workings and angular may have better luck.

when the record doesn't have any holds, the instruction to close the progress dialog manages to run before the dialog is displayed.

i tried using chrome's debugger, putting breakpoints on egProgressDialog.open (near line 1678) and egProgresDialog.close (near line 1714) in catalog/app.js

with a breakpoint on both opening and closing the dialog, the dialog is open when it is closed.
with a breakpoint only on closing the dialog, the dialog is not yet open when it is closed.

the stupid fix (now pushed to the existing branch here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/abellenir/lp1731272-record-reload) is to wrap the call to egProgressDialog.close in a setTimeout, delaying its invocation by pushing it to the end of the event queue.

[Jane Sandberg (sandbergja)]

Thanks for your branch, A. Bellenir! The progress bar issue has been pretty frustrating for our staff, so I really appreciate you working on it.

However, it seems like the stuck progress bar is still happening with your branch user/abellenir/lp1731272-record-reload. Here are my steps using the concerto data set:

1) Create a new workstation at BR3.
2) Search for 223
3) Select record #223 (Throne of the Crescent Moon)
4) Click on the View Holds tab

[a. bellenir (abellenir)]

Jane,

i wasn't able to replicate the error. :-(

i wonder if a code merge could've removed the relevant setTimeout javascript call from your installation.

i got a merge conflict in Open-ILS/web/js/ui/default/staff/cat/catalog/app.js when getting this branch caught up with master. among other changes, the code switched from bracket notation for accessing the 'finally' property:
    )['finally'](egProgressDialog.close)
to dot notation:
    ).finally(egProgressDialog.close)

the important bit here is that the egProgressDialog.close should* be wrapped in a setTimeout, which could've been misplaced if there was a merge conflict with the branch:
    ).finally(function(){ setTimeout(egProgressDialog.close); });

i merged master's latest changes into this branch and updated it. hopefully it won't have any more conflicts.

* i say "should", but this is arguable. using setTimeout to manipulate the javascript event queue as a means of working around race conditions in angular promises isn't exactly "elegant". i just couldn't figure out a better solution.

[Jennifer Pringle (jpringle-u)]

I tested this on the 3.1.5 bug squashing sandbox and I no longer see the status bar hanging for records with holds or without holds.

[Dan Wells (dbw2)]

I believe this bug as written is solved by the fix on #1797923, so marking this as a duplicate. Bug #1724348 remains as a separate issue, and some of this discussion and branch may apply over there, so I will comment over there as well.