Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/fonts behind reverse proxy when minify = true #4002

Merged
merged 2 commits into from
May 14, 2020
Merged

Fix/fonts behind reverse proxy when minify = true #4002

merged 2 commits into from
May 14, 2020

Conversation

muxator
Copy link
Contributor

@muxator muxator commented May 14, 2020

This is a proposal to fix #3956.

There is:

  1. a problem of cache busting in ace.js left open by 95fd5ce;
  2. 4177b3f moved the font-face declarations from src/static/css/pad.css to two
    imported files in a different directory, and this requires a rebase on clean-css.

Tested the following 4 combinations with Firefox 75 on linux:

  • minify: true|false

  • directly invoking http://localhost:9001/p/somepad and http://localhost/pad/foo/p/somepad using the following nginx configuration:

    server {
            listen       80;
            server_name  pad.example.com;
    
            access_log  /var/log/nginx/eplite.access.log;
            error_log   /var/log/nginx/eplite.error.log;
    
        location /pad/foo {
            rewrite                /pad/foo/(.*) /$1 break;
            rewrite                ^/pad/foo$ /pad/foo permanent;
            proxy_pass             http://localhost:9001/;
            proxy_pass_header Server;
            proxy_redirect         / /pad/foo;
            proxy_set_header       Host $host;
            proxy_buffering off;
        }
    
        location /pad/foo/socket.io {
            rewrite /pad/foo/socket.io/(.*) /socket.io/$1 break;
            proxy_pass http://localhost:9001/;
            proxy_redirect         / /pad/foo/;
            proxy_set_header Host $host;
            proxy_buffering off;
            proxy_set_header Host $host;  # pass the host header
            proxy_http_version 1.1;  # recommended with keepalive connections
            # WebSocket proxying - from http://nginx.org/en/docs/http/websocket.html
            proxy_set_header Upgrade $http_upgrade;
            proxy_set_header Connection $connection_upgrade;
        }
    }

muxator added 2 commits May 14, 2020 01:54
…m acs.js

Before this change, a client would require two versions of the same assets (with
and without randomVersionString), wasting resources and triggering all sorts of
hard to debug inconsistencies.

This change should have been part of 95fd5ce and completes it.
4177b3f moved the font-face declarations from src/static/css/pad.css to two
imported files (src/static/css/pad/fonts.css, src/static/css/pad/toolbar.css)
in a different directory.

This results in the font files being invoked from CSSes residing in different
directories in the minified and un-minified case. URLs in the src attribute are
relative to the stylesheet path [0], and so we have to start requiring clean-css
to rebase them.

Before this change, the non minified casse worked by chance, because there were
a lot of "..", which ended up resolving to the root of the site anyways.

Fixes #3956

[0] https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/src
@julpec
Copy link

julpec commented May 14, 2020

Hello
It's the first time I want to test a PR. I really don't know how to do it even following github's doc. If you could give me the git fetch line that I can't build. Thanks

@muxator muxator changed the title Fix/fonts behind reverse proxy Fix/fonts behind reverse proxy when minify = true May 14, 2020
@muxator muxator mentioned this pull request May 14, 2020
@muxator
Copy link
Contributor Author

muxator commented May 14, 2020

This PR fixes a bug, improves cache hit ratio, reduces the possibility of being hit by more bugs due to stale assets, but - on the other side - introduces more HTTP requests.

For an analysis, see #3956 (comment).

Pulling in.

@muxator muxator merged commit 37abb21 into ether:develop May 14, 2020
@muxator muxator deleted the fix/fonts-behind-reverse-proxy branch May 14, 2020 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants