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

[css-viewport-1] Add automation support for viewport segments #11137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

darktears
Copy link
Contributor

This commits define ways to emulate display features of a device. A display feature is a hardware feature that acts as a divider and creates logically separate regions of the viewport.

Using WebDriver developers and user agents will have the opportunity to test or emulate several types of devices.

@darktears darktears force-pushed the viewport-segments-testing branch 2 times, most recently from 9f414a0 to a58630b Compare November 1, 2024 15:13
@bramus
Copy link
Contributor

bramus commented Nov 3, 2024

I don’t think I am the right person to co-review this. I would assume the authors of the https://w3c.github.io/webdriver/ spec are the people you are looking for.

@darktears
Copy link
Contributor Author

darktears commented Nov 4, 2024

@OrKoN would you be able to review this please?

The approach is modeled after Compute Pressure, Device Posture and Generic Sensor.

@OrKoN
Copy link

OrKoN commented Nov 4, 2024

Thanks for the ping, let me try to review it:

  1. Is there a rendered version that includes changes?
  2. have you considered specifying it as part of https://w3c.github.io/webdriver-bidi/#command-browsingContext-setViewport instead?
  3. is anything expected to happen when [=[[DisplayFeaturesOverride]]=] is set? currently, it seems the override is set for the future use by the algorithm and the command returns?

@darktears
Copy link
Contributor Author

@OrKoN

  1. If by rendered, you mean after bikeshed is run, not yet. I can find a way to throw the generated html somewhere for you to see.
  2. No but I'm wondering if it's a good fit. setViewport command only allows to set the width/height and devicePixelRatio but do not seem to include any other viewport properties. If we decide to add a parameter in setViewport then where would all the explanations and diagrams go?
  3. I don't understand your question. Setting [=[[DisplayFeaturesOverride]]=] is the very purpose of this. When the override is set then the segments property is recalculated using the array of display features from DisplayFeaturesOverride slot coming from the setdisplayfeatures command.

@OrKoN
Copy link

OrKoN commented Nov 4, 2024

If by rendered, you mean after bikeshed is run, not yet. I can find a way to throw the generated html somewhere for you to see.

Yes, usually https://github.com/w3c/spec-prod is used to publish a preview but looks like it is not set up in this repo.

No but I'm wondering if it's a good fit. setViewport command only allows to set the width/height and devicePixelRatio but do not seem to include any other viewport properties. If we decide to add a parameter in setViewport then where would all the explanations and diagrams go?

I assume it works the same way (it is also Bikeshed-based), in a non-normative section, with files being put to a folder. Note that I am not saying it is required.

I don't understand your question. Setting [=[[DisplayFeaturesOverride]]=] is the very purpose of this. When the override is set then the segments property is recalculated using the array of display features from DisplayFeaturesOverride slot coming from the setdisplayfeatures command.

Probably I made an assumption that is not correct. If this command is only meant to override the segments getter available to the page, the current steps look sufficient. Or does the command imply a way to emulate the viewport with segments visually? Is the command expected to also trigger relevant Screen Orientation, Window Resize or Posture change events mentioned in the spec or any other events? And to confirm the DOM attribute cannot be overriden from JavaScript for testing?

@darktears
Copy link
Contributor Author

darktears commented Nov 4, 2024

Yes, usually https://github.com/w3c/spec-prod is used to publish a preview but looks like it is not set up in this repo.

Yes, I'm not entirely sure why. I'm not really a regular contributor to the CSS WG. I just happen to work on adding the viewport segments functionality.

I assume it works the same way (it is also Bikeshed-based), in a non-normative section, with files being put to a folder. Note that I am not saying it is required.

I meant more the fact that the BIDI spec doesn't really cover the concepts but just defines the commands. For me it felt natural to have the Automation story sit next to the property it covers. Also the viewport segments concept goes hand in hand with the Device Posture API and we have dedicated commands for the Device Posture API. I don't have a strong opinion.

If this command is only meant to override the segments getter available to the page, the current steps look sufficient.

This is correct. Please note that the entire viewport segments concept also defines MQs and env variables which will also be invalidated and recalculated. After this patch lands, I was planning to reference this addition on the MQ spec and CSS variable spec to cover the [DisplayFeaturesOverride] case.

Or does the command imply a way to emulate the viewport with segments visually? Is the command expected to also trigger relevant Screen Orientation, Window Resize or Posture change events mentioned in the spec or any other events?

Screen orientation, Window Resize and posture change are not expected to change when the internal slot is set.

And to confirm the DOM attribute cannot be overriden from JavaScript for testing?

The segments property is a read only frozen array so no (unless I'm mistaken), MQs can't be overridden but CSS env variables could I believe. Besides the automation aspect for authors to try and automate testing of their websites on different foldable devices the intention of this patch is also for testing purposes for user agents. If we can override the display features coming from the Windows and Android native APIs we can also verify the calculation of the segments property, the CSS MQs and the CSS Env variables through various configurations (more than one fold for example). This will be very useful for conformance.

@OrKoN
Copy link

OrKoN commented Nov 4, 2024

That sounds good.The PR looks good to me % minor comments.
Do you plan to add WPTs for these WebDriver methods in https://github.com/web-platform-tests/wpt/tree/master/webdriver/tests/classic?

Copy link

@OrKoN OrKoN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

css-viewport-1/Overview.bs Outdated Show resolved Hide resolved
css-viewport-1/Overview.bs Outdated Show resolved Hide resolved
css-viewport-1/Overview.bs Outdated Show resolved Hide resolved
@darktears
Copy link
Contributor Author

That sounds good.The PR looks good to me % minor comments. Do you plan to add WPTs for these WebDriver methods in https://github.com/web-platform-tests/wpt/tree/master/webdriver/tests/classic?

I intend to do something similar to https://github.com/web-platform-tests/wpt/tree/master/device-posture and add tests to verify the segments property, CSS MQs and environment variables.

@OrKoN
Copy link

OrKoN commented Nov 4, 2024

It would be also nice to add WebDriver-specific tests to test error conditions, wrong input, some happy path cases. Some browsers might not actually implement testdriver.js using WebDriver so it is also a good indication for external users for the level of support in the corresponding driver.

@darktears
Copy link
Contributor Author

It would be also nice to add WebDriver-specific tests to test error conditions, wrong input, some happy path cases. Some browsers might not actually implement testdriver.js using WebDriver so it is also a good indication for external users for the level of support in the corresponding driver.

I can investigate that.

This commits define ways to emulate display features of
a device. A display feature is a hardware feature that acts
as a divider and creates logically separate regions of the
viewport.

Using WebDriver developers and user agents will have the
opportunity to test or emulate several types of devices.
@darktears darktears force-pushed the viewport-segments-testing branch from a58630b to bc97df6 Compare November 4, 2024 21:03
@darktears
Copy link
Contributor Author

I will still need a reviewer like @emilio to approve and merge this change. Thank you!

@emilio
Copy link
Collaborator

emilio commented Nov 6, 2024

Is there a precedent to put these kind of spec text inside the CSS spec? I haven't seen it before... cc @fantasai @tabatkins

@OrKoN
Copy link

OrKoN commented Nov 6, 2024

I have not seen a precedent with CSS specs but here other specs defining WebDriver extension commands https://dontcallmedom.github.io/webdex/e.html#extension%20commands%40%40webdriver%25%25dfn (not sure if the tool omits some specs)

@darktears
Copy link
Contributor Author

WebAuth, Device Posture, Compute Pressure, Generic Sensors, Web Bluetooth for example define the automation/testing story in their respective specs.

Some specs like WebUSB are defining them into a dedicated spec, some specs into an explainer like WebXR.

So, I don't think there is a W3C guidance around that.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Nov 27, 2024

The CSS Working Group just discussed [css-viewport-1] Add automation support for viewport segments.

The full IRC log of that discussion <chrishtr> Rossen: is Alexis Menard on the call?
<chrishtr> emilio: I can introduce it otherwise
<chrishtr> rossen: otherwise we can move on
<chrishtr> emilio: there is no precedent for specifying web driver things in CSS specs, is this something we want to do?
<chrishtr> emilio: that is the main question
<chrishtr> rossen: I looked for issues on this topic beside the PR and didn't find one
<chrishtr> rossen: one path forward is to say we don't have precedent or reason and to say no to web driver things in our specs
<chrishtr> emilio: would it be reasonable to file an issue with the meta point?
<chrishtr> rossen: agreed
<chrishtr> emilio: will file an issue
<Rossen1> ack fantasai
<chrishtr> fantasai: there are also editorial improvements in this PR that should land regardless
<chrishtr> fantasai: recommend separating those two things from the web driver into separate PRs
<chrishtr> rossen: ok, so split the PR into editorial and web driver, and emilio will open an issue for the meta point

Summary:

  • Split out editorial improvements from PR and land those, so the PR is only about the WebDriver points.
  • Open a new issue to discuss integration with WebDriver and this PR.

@darktears
Copy link
Contributor Author

@emilio what you want me to land for editorial improvements?

The DisplayFeature part with its relevant diagram? The concept is really linked to the automation part.

For sure the tab fix on the IDL of the segments property? Anything else?

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

Successfully merging this pull request may close these issues.

5 participants