Skip to content

Extend geolocation tests #52243

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

Merged
merged 5 commits into from
May 12, 2025
Merged

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented Apr 29, 2025

Copy Chromium's wpt-internal test to public WPT using testdriver geolocation API.

});

// Ensure any previously set geolocation emulations are cleared.
await test_driver.bidi.emulation.set_geolocation_override(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have agreement on the shape of the BiDi API? @reillyeon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

w3c/geolocation#183 was merged based on consensus between Chromium and Firefox.

Copy link
Contributor

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Some of these things are a bit redundant or not quite correct and potentially racy.

@sadym-chromium sadym-chromium force-pushed the sadym/geolocation-internal-wpt branch from af97157 to 55fec53 Compare April 30, 2025 08:09
@sadym-chromium sadym-chromium force-pushed the sadym/geolocation-internal-wpt branch from 55fec53 to 595d3c7 Compare May 5, 2025 14:06
@sadym-chromium
Copy link
Contributor Author

@marcoscaceres please take a look

@sadym-chromium sadym-chromium enabled auto-merge (squash) May 6, 2025 06:34
@sadym-chromium sadym-chromium force-pushed the sadym/geolocation-internal-wpt branch from 595d3c7 to 800ff96 Compare May 6, 2025 16:43
@sadym-chromium sadym-chromium force-pushed the sadym/geolocation-internal-wpt branch from 800ff96 to dbf6924 Compare May 6, 2025 16:53
@sadym-chromium sadym-chromium dismissed marcoscaceres’s stale review May 12, 2025 14:46

I assume the concerns raised by @marcoscaceres are addressed

@sadym-chromium sadym-chromium merged commit c647833 into master May 12, 2025
17 checks passed
@sadym-chromium sadym-chromium deleted the sadym/geolocation-internal-wpt branch May 12, 2025 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants