Skip to content

In emulation.setMediaFeaturesOverride the null case is not handled properly #1168

Description

@juliandescottes

Noticed some issues with setMediaFeaturesOverride.

1. Wrong access to map iterate keys/values

We iterate on the map as follows:

1. For each |media feature| of |media features override|:
   1. [=map/Set=] |media features override map|[|media feature|["<code>name</code>"]] to
      |media feature|["<code>value</code>"].

But |media feature|["name"] and |media feature|["value"] don't exist.
I think we usually assume that when we use for each on a map we get the value only. If we want to read the key we should use the form suggested at https://infra.spec.whatwg.org/#map-iterate:

1. For each |media feature name| → |media feature value| of |media features override|:
   1. [=map/Set=] |media features override map|[|media feature name|] to
      |media feature value|.

2. Null case handling

Null case is not properly storing unset in the configuration (and also tries to iterate on unset, which is a bug).

1. Let |media features override| be |command parameters|["<code>features</code>"].

1. If |media features override| is null, set |media features override| to
   [=WebDriver configuration/unset=].

1. Let |media features override map| be an empty [=/map=].

1. For each |media feature| of |media features override|:

If |media features override| is null, |media features override| is unset: we should directly store it in the configuration and avoid attempting to call For each on it.

3. Unclear error case for unsupported features

Current step 5 reads:

1. If the implementation does not support overriding any of the media features in
   |media features override map|, return [=error=] with [=error code=] [=unsupported operation=].

"does not support overriding any of the media features" is a bit unclear. It depends how not / any work together. Basically:

  • (not support)(any override) == 1 override is unsupported
  • (not)(support any override) == all overrides are unsupported

Can we switch to a clearer wording which doesn't leave room for interpretation?

cc @lutien

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingmodule-emulationEmulation module

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions