Skip to content

Fix an icon link that was generating bad URLs #2045

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

Merged
merged 1 commit into from
Dec 22, 2019

Conversation

jtgeibel
Copy link
Member

On current staging, but not production, navigating directly to a path
such as /crates/crate-name results in a request sent to
/crates/cargo-835dd6a18132048a52ac569f2615b59d.png (a 404 response
status).

It appears that the src field is now treated as a relative path.
Using an absolute URL works for me locally.

r? @Turbo87

On current staging, but not production, navigating directly to a path
such as `/crates/crate-name` results in a request sent to
`/crates/cargo-835dd6a18132048a52ac569f2615b59d.png` (a 404 response
status).

It appears that the `src` field is now treated as a relative path.
Using an absolute URL works for me locally.
@jtgeibel
Copy link
Member Author

I've pushed this to staging and this issue is gone there.

I've also noticed that navigating directly to a crate or version page results in a spinner in place of the follow button. For example: https://staging.crates.io/crates/egg-man

Things work after navigating within the app. It seems no request is sent to the backend endpoint when navigating directly.

@Turbo87
Copy link
Member

Turbo87 commented Dec 22, 2019

I assume this is related to zonkyio/ember-web-app#144, which got merged in #2002.

I'm wondering though if this is the correct resolution or if the image actually should be fingerprinted. I'll have a look.

@Turbo87
Copy link
Member

Turbo87 commented Dec 22, 2019

Since https://zonkyio.github.io/ember-web-app/docs/manifest/available-properties#icons is also using absolute paths I'll go ahead and merge this 👍

@bors r+

@bors
Copy link
Contributor

bors commented Dec 22, 2019

📌 Commit b3aac9b has been approved by Turbo87

@bors
Copy link
Contributor

bors commented Dec 22, 2019

⌛ Testing commit b3aac9b with merge ecba6b2...

bors added a commit that referenced this pull request Dec 22, 2019
Fix an icon link that was generating bad URLs

On current staging, but not production, navigating directly to a path
such as `/crates/crate-name` results in a request sent to
`/crates/cargo-835dd6a18132048a52ac569f2615b59d.png` (a 404 response
status).

It appears that the `src` field is now treated as a relative path.
Using an absolute URL works for me locally.

r? @Turbo87
@bors
Copy link
Contributor

bors commented Dec 22, 2019

☀️ Test successful - checks-travis
Approved by: Turbo87
Pushing ecba6b2 to master...

@bors bors merged commit b3aac9b into rust-lang:master Dec 22, 2019
@jtgeibel jtgeibel deleted the fix-bad-icon-link branch December 22, 2019 23:55
@jtgeibel
Copy link
Member Author

@Turbo87 from what I've seen, these files do end up hashed in production. I think we were good as of #1741, unless we've regressed somewhere. For instance, cargo.png is deployed as https://staging.crates.io/cargo-835dd6a18132048a52ac569f2615b59d.png.

@Turbo87
Copy link
Member

Turbo87 commented Dec 23, 2019

yep, saw that when I was trying out your branch locally :)

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

Successfully merging this pull request may close these issues.

4 participants