Browser client iframe (catalog, etc.) loading page

Bug #1797923 reported by Bill Erickson on 2018-10-15
24
This bug affects 8 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
3.1
Medium
Unassigned

Bug Description

Evergreen 3.2

In the AngJS browser client, the catalog and various other interfaces are displayed within an iframe. When the iframe is first inserted in the page, the value for the "src" attribute is empty, even when a "src" value has been provided to the egEmbedFrame controller. (Angular applies the "src" attribute after the iframe is added to the DOM).

Because if this, the iframe tries to load a copy of the current page (empty src == "here"), which often results in seeing nested versions of the AngJS app while the page that should be loading is waiting to load.

If the system is slow enough to respond, this nesting can occur by simply navigating to a record detail page (e.g. /eg/staff/cat/catalog/record/1).

This may be a duplicate of bug #1731272.

See also bug #1731305.

Not only is this confusing, it means the browser is working extra hard to re-render a duplicate AngJS environment.

To prevent the iframe from attempting to load anything other than the desired page at load time, I propose the iframe default to the URL of a simple loading page instead of an empty string. This avoids the unnecessary page load and makes it obvious to users that a page is still loading.

Branch on the way.

Bill Erickson (berick) wrote :

Patch:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1797923-angjs-iframe-loading-page

Going to tag this for back-porting as well, since I consider this to be a bug.

Changed in evergreen:
milestone: none → 3.2.1
assignee: Bill Erickson (berick) → nobody
tags: added: pullrequest
Jason Boyer (jboyer) wrote :

I took this branch for a test drive; loader loads, spinner spins, and most importantly this does seem to have fixed both of the additional bugs mentioned in the initial report. I was able to set any tab as a default, load a record by id and go straight to that tab.

Signoff lives here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1797923-angjs-iframe-loading-signoff working/user/jboyer/lp1797923-angjs-iframe-loading-signoff

tags: added: signedoff
Galen Charlton (gmc) on 2018-10-23
Changed in evergreen:
importance: Undecided → Medium
status: New → Confirmed
assignee: nobody → Galen Charlton (gmc)
Dan Wells (dbw2) wrote :

I also tested this and overall it looks good to me, but since multiple tests are in progress, I won't bother with another sign-off branch. A few things to note:

1) Probably unique to some condition of my system, but in testing, about every 5th or so catalog load resulted in a dead screen with a "Error loading shared worker" error in the console. Again, I don't think it was related to this change, but throwing it out there just in case.

2) "Default tab" behavior is still not quite what I would expect. Going straight to the record by ID does indeed work, but loading records from a catalog search does not work for me. I know it wasn't the focus of this bug, so not a reason to hold off.

Thanks, Bill!

Dan

Jason Boyer (jboyer) wrote :

Regarding Dan's second point: if you set a default tab other than OPAC view, that default isn't honored when following a record link from search results, only when loading a record directly. checking in the XUL client it looks like this did work, though it still loaded the OPAC view first.

Like he said, that's not the point of this bug, but I just wanted to add a little clarification since the state of bug 1731272 will be greatly improved, but not 100% fixed.

Galen Charlton (gmc) wrote :

I see the same things regarding the treatment of the default tab, but since this is a solid step forward and otherwise tests OK for me, I've pushed this to master, rel_3_2, and rel_3_1. Thanks, Bill, Jason, and Dan!

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
status: Confirmed → Fix Committed
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.

Duplicates of this bug

Other bug subscribers