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

Preserve domain of reloaded assets #1218

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

aiba
Copy link
Contributor

@aiba aiba commented Feb 19, 2025

This preseves domain of static assets that devtools matches in the case that those assets are served off a different domain from the page.

Thank you @swannodette for guidance!

@thheller
Copy link
Owner

Can you explain your setup? I mean doesn't the prior check in the modified and still prevent the reload? It specifically compares the domains or the page initially and the css? Is the HTML also coming from another domain?

@swannodette
Copy link

@thheller yeah, in local development we're serving CSS assets from a different domain than the HTML, so that reloading stopped working w/o this change. I think this change is backwards compatible but maybe we missed something?

@thheller
Copy link
Owner

That didn't really answer my question.

      (and (or (= (.hasSameDomainAs page-load-uri node-uri))
               (not (.hasDomain node-uri)))
           (= node-abs new)
           new)

The code in question uses the old URI (which is the href attribute of the link node being checked). It then checks whether (.hasSameDomainAs page-load-uri node-uri) or the URI didn't have a domain at all. From what you are saying I'm gathering that the link tag contains a full http:.. URL pointing to a different domain. Thus this check would fail if the initial HTML isn't also coming from that other domain, which you said is not the case? So, either the current code isn't working as it was intended at all, or your fix is missing something I do not understand about your setup?

I do not mind adding this. I just would like to understand how it actually fixes anything?

@swannodette
Copy link

Not pointing to completely different domain - a subdomain, i.e.:

(.hasSameDomainAs (goog.Uri/parse "static.foo") (goog.Uri/parse "foo")) ;; true

@thheller thheller merged commit 9141450 into thheller:master Feb 21, 2025
1 check passed
@thheller
Copy link
Owner

ah, that makes sense. Should be fixed as of 2.28.21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants