Skip to content
This repository has been archived by the owner on Apr 10, 2018. It is now read-only.

sematics for mapbox:// glyphs urls in v8 #309

Closed
mikemorris opened this issue Jul 2, 2015 · 16 comments
Closed

sematics for mapbox:// glyphs urls in v8 #309

mikemorris opened this issue Jul 2, 2015 · 16 comments
Assignees
Milestone

Comments

@mikemorris
Copy link

Specify owner in URL rather than trying to guess this based on an access token or the owner of a project using the style. Would allow explicitly using shared, public mapbox fonts in addition to private user fonts.

@mikemorris
Copy link
Author

This is what I was thinking about today that could possibly be a blocker for implementing user-uploaded fonts @samanpwbb @tmcw @jakepruitt

@samanpwbb
Copy link
Contributor

I don't think this is necessary. We can hardcode the username directly into the fontstack URL in our styles. So in my styles, you would see:

"glyphs" : "https://api.mapbox.com/saman/fonts/{fontstack}/{range}.pbf"

or whatever the final API endpoint for fonts is. My username can be hardcoded into the stylesheet.

@jakepruitt
Copy link

In order to add the access token, we're going to keep with the mapbox:// prefixed glyphs url, and allow for urls like

mapbox://fonts/v1/saman/{fontstack}/{range}.pbf

to access the new API at

https://api.mapbox.com/fonts/v1/saman/Arial Unicode MS Bold/0-255.pbf?access_token=[accesstoken]

Legacy support will still be maintained for any glyphs urls starting with mapbox://fontstack....

jakepruitt pushed a commit to mapbox/mapbox-gl-js that referenced this issue Jul 23, 2015
- does not prepend /v4/ to api endpoints like /fonts/v1/
- only prepends it to things starting with /fontstack
- solves the problem in #1385
- refs mapbox/mapbox-gl-style-spec#309
jakepruitt pushed a commit to mapbox/mapbox-gl-js that referenced this issue Jul 23, 2015
- does not prepend /v4/ to api endpoints like /fonts/v1/
- only prepends it to things starting with /fontstack
- solves the problem in #1385
- refs mapbox/mapbox-gl-style-spec#309
jakepruitt pushed a commit to mapbox/mapbox-gl-native that referenced this issue Jul 23, 2015
- does not prepend /v4/ to api endpoints like /fonts/v1/
- only prepends it to things starting with /fontstack
- solves the problem in #1918
- refs mapbox/mapbox-gl-style-spec#309
jakepruitt pushed a commit to mapbox/mapbox-gl-native that referenced this issue Jul 27, 2015
- does not prepend /v4/ to api endpoints like /fonts/v1/
- only prepends it to things starting with /fontstack
- solves the problem in #1918
- refs mapbox/mapbox-gl-style-spec#309
@jfirebaugh
Copy link
Contributor

The format mapbox://fonts/v1/{user}/{fontstack}/{range}.pbf is slightly inconsistent with other mapbox:// URLs, which by convention do not include the resource type or version number in the path. E.g.:

Style: mapbox://user.idhttp://api.mapbox.com/styles/v1/user.id?access_token=...
Source: mapbox://user.idhttp://api.mapbox.com/v4/user.id.json?access_token=...

Arguably this is a design mistake which a format like mapbox://fonts/v1/... rectifies. The argument being that URLs should uniquely refer to a single resource, and so either the resource type should be included, like mapbox://styles/user.id, or each resource type should have a separate scheme, like mapbox-style://user.id.

I'm not sure whether the version specifier should be included or not. If it is included, then we would likely not be able to use a new API version in GL without also making a style spec version bump. But that may not be possible in practice anyway.

@jfirebaugh jfirebaugh changed the title Specify font owner in style? sematics for mapbox:// glyphs urls in v8 Aug 4, 2015
@jfirebaugh jfirebaugh added this to the v8 milestone Aug 4, 2015
@jakepruitt
Copy link

@willwhite I know that you had an argument against specifying the version URL in the style, possibly for migration reasons. I agree with @jfirebaugh that this should be specified in the style spec in some way to prevent more breaking changes from occurring due to the ambiguity of the mapbox:// interpretation. @jfirebaugh how much time would it take to change to the mapbox-styles:// or mapbox://styles formats to make things homogeneous?

@jfirebaugh
Copy link
Contributor

We could switch to mapbox://styles/{style-id} in v8 without too much difficulty. We should do the same for sources, though it's a bit odder because sources live under the legacy URL scheme rather than at /sources/v1/.... But I'm comfortable having mapbox://sources/{source-id} mapping to legacy URLs for now and changing to /sources/v1/... when that exists.

@jakepruitt
Copy link

In that case, we are fine to keep mapbox://fonts/v1 for the glyphs URL?

@jfirebaugh
Copy link
Contributor

I'm leaning towards leaving the version out of all URLs. Makes it easier to make the switch for sources in the future.

@jakepruitt
Copy link

I like the sound of that plan, allows for more flexibility and less migrations of style documents. Let me know if I can help with anything with styles. I think my PR's on gl-js and gl-native can handle the version removal on the client side, we'll just need to push the change to our style documents.

@tmcw tmcw mentioned this issue Aug 10, 2015
82 tasks
@lucaswoj
Copy link

Is this a good summary of the changes that need to happen? @jfirebaugh @jakepruitt @samanpwbb

  • the style migrator needs to
    • rewrite mapbox font urls to mapbox://font/user.id
    • rewrite mapbox source urls to mapbox://source/user.id
  • gl-js and gl-native need to
    • accept mapbox style urls as mapbox://style/user.id
    • accept mapbox font urls as mapbox://font/user.id
    • accept mapbox source urls as mapbox://source/user.id
  • api needs to
    • serve mapbox styles from mapbox://style/user.id
    • serve mapbox fonts from mapbox://font/user.id
    • serve mapbox sources from mapbox://source/user.id
  • gl-styles needs to be migrated

@jfirebaugh
Copy link
Contributor

@lucaswoj Let's just do font URLs for now. There are a few things in play with source and style URLs that make me hesitate to change them just yet.

For fonts, it's:

"glyphs": "mapbox://fonts/user/{fontstack}/{range}.pbf"

Where user is an actual username and the placeholders are interpolated by the renderer at runtime.

@jfirebaugh
Copy link
Contributor

The migrator should rewrite both mapbox://fontstack/{fontstack}/{range}.pbf and mapbox://fonts/v1/user/{fontstack}/{range}.pbf to mapbox://fonts/user/{fontstack}/{range}.pbf. For the former, it's probably safe to assume the user is mapbox.

@lucaswoj
Copy link

@jfirebaugh Is this a good subtask list?

  • the style migrator needs to rewrite mapbox font urls to mapbox://fonts/[user]/{fontstack}/{range}.pbf
  • gl-js and gl-native need to rewrite mapbox font urls from mapbox://fonts/[user]/{fontstack}/{range}.pbf to https://api.mapbox.com/fonts/v1/...?access_token=...
  • gl-styles needs to be migrated

@jfirebaugh
Copy link
Contributor

  • api needs to serve mapbox fonts from mapbox://fonts/[user]/{fontstack}/{range}.pbf

Replace this with "gl-js and gl-native need to rewrite mapbox://fonts/[user]/{fontstack}/{range}.pbf to https://api.mapbox.com/fonts/v1/...?access_token=.... See e.g. https://github.com/mapbox/mapbox-gl-js/blob/215659260baf99b5a437c032677a5ad12e3747ea/js/util/mapbox.js#L54.

Otherwise, 👍.

@lucaswoj
Copy link

Got it! Thanks for the clarification.

@lucaswoj
Copy link

AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
- does not prepend /v4/ to api endpoints like /fonts/v1/
- only prepends it to things starting with /fontstack
- solves the problem in mapbox#1918
- refs mapbox/mapbox-gl-style-spec#309
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants