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

Zoom stop functions permit literal expressions, which trigger font API 404 status code #7387

Closed
emilymdubois opened this issue Oct 9, 2018 · 2 comments · Fixed by #7396
Closed
Labels

Comments

@emilymdubois
Copy link

emilymdubois commented Oct 9, 2018

mapbox-gl-js version: 0.46.0

browser: Google Chrome v69.0.3497.100

Steps to Trigger Behavior

  1. Create a stylesheet with a zoom stop function containing literal expressions (see snippet below).
  2. Observe that stylesheet passes validation.
  3. Zoom into/out of the map so that the zoom stops are breached.
  4. Observe API error in console with format https://api.mapbox.com/fonts/v1/{username}/literal,{encoded_fonts}/{range}.pbf?access_token={token}. It looks like the literal expression reference is making its way into the font request.
{
  "base": 1,
  "stops": [
    [
      6,
      [
        "literal",
        [
          "Open Sans Regular",
          "Arial Unicode MS Regular"
        ]
      ]
    ],
    [
      7,
      [
        "literal",
        [
          "Open Sans Bold",
          "Arial Unicode MS Regular"
        ]
      ]
    ]
  ]
}

Link to Demonstration

  1. Click on this style link.
  2. Observe font API error in browser network console.

Expected Behavior

The literal expression reference should be removed before making the font API request.

Actual Behavior

The font API request contains the literal expression reference.

@ansis
Copy link
Contributor

ansis commented Oct 9, 2018

It looks like your example is mixing the the old data-driven functions { base: 1, stops: [...]} with new expressions ["literal", ...]. We support mixing the within styles, but not within individual style properties.

I think you could rewrite it to use either just the old style, or, preferably the new. I haven't tested these examples so I could be adding some other bug:

Old:

{
  "base": 1,
  "stops": [
    [
      6,
      [
        "Open Sans Regular",
        "Arial Unicode MS Regular"
      ]
    ],
    [
      7,
      [
        "Open Sans Bold",
        "Arial Unicode MS Regular"
      ]
    ]
  ]
}

New (using step expressions):

["step",
    ["zoom"],
    ["literal", ["Open Sans Regular", "Arial Unicode MS Regular"],
    7, ["literal", ["Open Sans Bold", "Arial Unicode MS Regular"]]
]

Did you write your expression in Studio? Did studio provide any sort of validation. I'm wondering what we can do to help people avoid this issue.

@emilymdubois
Copy link
Author

Did you write your expression in Studio?

Yes. We've patched a fix that makes this difficult to do in the UI, but the JSON editor is pretty fair game.

Did studio provide any sort of validation.

Yes, we validate property values configured in the JSON editor using the mapbox-gl-js validate_property script. However, you can see that it's not catching this error:

screen shot 2018-10-09 at 1 04 19 pm

I can implement a catch in Studio, but if I'm accurate in thinking the validate_property script should throw an error on this, I can submit the pull request into this repo directly.

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

Successfully merging a pull request may close this issue.

2 participants