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

"translations into a ..." links are pointing to the wrong URL - Meta viewport allows for zoom (proposed) - b4f0c3 #2170

Open
giacomo-petri opened this issue Mar 29, 2024 · 3 comments

Comments

@giacomo-petri
Copy link
Collaborator

Currently, the links within the notes of the Expectations section of Meta viewport allows for zoom rule, direct to the wrong resource: https://www.w3.org/TR/css-viewport-1/, where the algorithm differs significantly from the one used to calculate failing examples.

The new algorithm is as follows:
Set-Property matches the listed property names case-insensitively. The property-value strings are interpreted as follows:

  1. If a prefix of property-value can be converted to a number using strtod, the value will be that number. The remainder of the string is ignored.
  2. If the value can not be converted to a number as described above, the whole property-value string will be matched with the following strings case-insensitively: yes, no, device-width, device-height
  3. If the string did not match any of the known strings, the value is unknown.

In contrast, the previous algorithm operated as follows:

  1. Non-negative number values are translated to values, clamped to the range [0.1, 10]
  2. Negative number values are dropped
  3. yes is translated to 1
  4. device-width and device-height are translated to 10
  5. no and unknown values are translated to 0.1

Given that the rule is tailored to old devices (as newer devices typically ignore settings like user-scalable:no and maximum-scale:1), it seems necessary to point to the specific version of the CSS Device Adaptation Module Level 1. Without this specificity, the current wording of the rule lacks coherence.

If you concur with this assessment, I'm happy to create a PR to implement these amendments.

@giacomo-petri
Copy link
Collaborator Author

I'll also seize this opportunity to eliminate Failed Example 6, as it duplicates Failed Example 5.

@giacomo-petri giacomo-petri self-assigned this Apr 30, 2024
@giacomo-petri
Copy link
Collaborator Author

Failed Example 6 has already been removed via a previous PR, rendering the previous comment unnecessary.

@Jym77 @carlosapaduarte, regarding my initial point, I'd like to hear your thoughts. While addressing this, I noticed that other links are also outdated, with anchors no longer functional.

I'm contemplating whether it would be prudent to redirect all these links to the "previous" (it's still the current official one) version of CSS Device Adaptation Module Level 1 or if we should amend the rule based on the new CSS Viewport Module Level 1 working draft. However, considering the meta is specific to older devices, perhaps the rule no longer holds relevance if we point to the new documentation.

@giacomo-petri giacomo-petri removed their assignment Apr 30, 2024
@dd8
Copy link
Collaborator

dd8 commented Dec 16, 2024

I think the rule should probably work off how meta viewport is implemented in browsers, rather than the spec, since there are important differences between the spec and implementations.

The Device Adaptation spec was reversed-engineered from the WebKit code - here's the actual code used in the current version of Safari.

void setViewportFeature(ViewportArguments& arguments, StringView key, StringView value, const ViewportErrorHandler& errorHandler)
{
    InternalViewportErrorHandler internalErrorHandler = [&errorHandler] (ViewportErrorCode errorCode, StringView replacement1, StringView replacement2) {
        errorHandler(errorCode, viewportErrorMessage(errorCode, replacement1, replacement2));
    };

    if (equalLettersIgnoringASCIICase(key, "width"_s))
        arguments.width = findSizeValue(key, value, internalErrorHandler, &arguments.widthWasExplicit);
    else if (equalLettersIgnoringASCIICase(key, "height"_s))
        arguments.height = findSizeValue(key, value, internalErrorHandler);
    else if (equalLettersIgnoringASCIICase(key, "initial-scale"_s))
        arguments.zoom = findScaleValue(key, value, internalErrorHandler);
    else if (equalLettersIgnoringASCIICase(key, "minimum-scale"_s))
        arguments.minZoom = findScaleValue(key, value, internalErrorHandler);
    else if (equalLettersIgnoringASCIICase(key, "maximum-scale"_s))
        arguments.maxZoom = findScaleValue(key, value, internalErrorHandler);
    else if (equalLettersIgnoringASCIICase(key, "user-scalable"_s))
        arguments.userZoom = findBooleanValue(key, value, internalErrorHandler);
#if PLATFORM(IOS_FAMILY)
    else if (equalLettersIgnoringASCIICase(key, "minimal-ui"_s)) {
        // FIXME: Ignore silently for now. This code should eventually be removed
        // so we start giving the warning in the web inspector as for other unimplemented keys.
    }
#endif
    else if (equalLettersIgnoringASCIICase(key, "shrink-to-fit"_s))
        arguments.shrinkToFit = findBooleanValue(key, value, internalErrorHandler);
    else if (equalLettersIgnoringASCIICase(key, "viewport-fit"_s))
        arguments.viewportFit = parseViewportFitValue(key, value, internalErrorHandler);
    else
        internalErrorHandler(ViewportErrorCode::UnrecognizedViewportArgumentKey, key, { });
}

static bool findBooleanValue(StringView key, StringView value, const InternalViewportErrorHandler& errorHandler)
{
    // yes and no are used as keywords.
    // Numbers >= 1, numbers <= -1, device-width and device-height are mapped to yes.
    // Numbers in the range <-1, 1>, and unknown values, are mapped to no.

    if (equalLettersIgnoringASCIICase(value, "yes"_s))
        return true;
    if (equalLettersIgnoringASCIICase(value, "no"_s))
        return false;
    if (equalLettersIgnoringASCIICase(value, "device-width"_s))
        return true;
    if (equalLettersIgnoringASCIICase(value, "device-height"_s))
        return true;
    return std::abs(numericPrefix(key, value, errorHandler)) >= 1;
}

The Mozilla code for parsing meta viewport works completely differently (nothing like the spec) and treats user-scalable=0, user-scalable=no, user-scalable=false as disabling zoom.

      mAllowZoom = true;
      if ((metaData.mUserScalable.EqualsLiteral("0")) ||
          (metaData.mUserScalable.EqualsLiteral("no")) ||
          (metaData.mUserScalable.EqualsLiteral("false"))) {
        mAllowZoom = false;
      }

https://searchfox.org/mozilla-central/source/dom/base/Document.cpp#10938

And the Chromium code is here, which is fairly close to the Webkit code, but it's not clear what ParsePositiveNumber below does when it sees user-scalable=-0.1

bool HTMLMetaElement::ParseViewportValueAsUserZoom(
    Document* document,
    bool report_warnings,
    const String& key_string,
    const String& value_string,
    bool& computed_value_matches_parsed_value) {
  // yes and no are used as keywords.
  // Numbers >= 1, numbers <= -1, device-width and device-height are mapped to
  // yes.
  // Numbers in the range <-1, 1>, and unknown values, are mapped to no.

  computed_value_matches_parsed_value = false;
  if (EqualIgnoringASCIICase(value_string, "yes")) {
    computed_value_matches_parsed_value = true;
    return true;
  }
  if (EqualIgnoringASCIICase(value_string, "no")) {
    computed_value_matches_parsed_value = true;
    return false;
  }
  if (EqualIgnoringASCIICase(value_string, "device-width"))
    return true;
  if (EqualIgnoringASCIICase(value_string, "device-height"))
    return true;

  float value =
      ParsePositiveNumber(document, report_warnings, key_string, value_string);
  if (fabs(value) < 1)
    return false;

  return true;
}

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_meta_element.cc;drc=f5e280b6c11f41545154266cd2317d3c730e9b30;l=265?q=user-scalable%20lang:cpp&ss=chromium

This means there are implementation differences:

user-scalable=no (disables zoom in Chrome, Safari and Firefox)
user-scalable=NO (disables zoom in Chrome and Safari, but not in Firefox)
user-scalable=false (disables zoom in Firefox, but also Chrome and Safari because false is an unknown value mapped to no)
user-scalable=0 (disables zoom in Chrome, Safari and Firefox)
user-scalable=0.1 (disables zoom in Chrome and Safari, but not in Firefox)

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

No branches or pull requests

2 participants