NPLD terms banner is hard to view and acceptance is modifying the URL

ukwa / ukwa-pywb

GNU General Public License v3.0

11 stars 3 forks source link

NPLD terms banner is hard to view and acceptance is modifying the URL #110

Closed anjackson closed 1 year ago

anjackson commented 1 year ago

Report that the NPLD terms banner is not viewable in some browsers. In particular, it's too large to see and vertical scrolling is apparently not working.

Also, accepting the terms seems to be modifying the URL, adding a ?terms-conditions=screen-reader suffix (when it should only set a cookie). This was in the NPLD Player.

Other issues arising during exploration:

Force Reload button goes back to default hardcoded homepage (readingroom.ldls.org.uk) not the ENV VAR one.
Seems NPLD_PLAYER_ALLOW_DEVTOOLS=true can't be specified at runtime, which is right, really, but makes debugging harder.
In closePopup, we should not call preventDefault when the form submission event is undefined.

anjackson commented 1 year ago

Hm, can't reproduce this. NPLD Player on Windows (BL Laptop) works as expected. Very odd.

anjackson commented 1 year ago

Aside, I think, but this can be tidied up. On a second viewing of a page:

(index):255 Uncaught TypeError: Cannot read properties of undefined (reading 'preventDefault')
    at closePopup ((index):255:7)
    at setModalVisibility ((index):264:7)
    at (index):268:3

i.e. the same logic is used to close then popup after accepting it, so preventDefault should not be called when the event is undefined.

anjackson commented 1 year ago

All very odd. Replied with more detail and asking for more detail!

anjackson commented 1 year ago

Image of problematic view looks like CSS/JS is being blocked, but I don't understand why that would be. I've run NPLD Player locally with DevTools and verified that the security header is being passed, and it is.

anjackson commented 1 year ago

Ah, I think this is because the embedded resources are being served over HTTP, because nothing is configuring the scheme/protocol.

Rather than hard-coding the deployment host or protocol, we use the convention of expecting particular headers to be set by upstream proxies. In particular, in NGINX, we use:

            proxy_set_header        X-Forwarded-Proto       $scheme;
            proxy_set_header        X-Forwarded-Host        $host;

(From https://github.com/ukwa/ukwa-services/blob/aa95df6854382e6b6e84edc697dcb4da2804ef9c/ingest/w3act/README.md?plain=1#L82-L83)

The web application picks those headers up and uses them to construct URLs that refer to itself. I think those are not set in this case, and the defaults are not working.

anjackson commented 1 year ago

Yep, fixing that seemed to make it work.