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

Remove Edge as a default product, but add breadcrumb link #2431

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

stephenmcgruer
Copy link
Contributor

@stephenmcgruer stephenmcgruer commented Mar 4, 2021

After a lot of discussion, we have decided to remove Edge as a default
product from wpt.fyi due to overlap in browser engine between Chrome and
Edge. In a perfect world we would invest in per-user default products,
but there is no resourcing for that currently.

To try to minimize the disruption, we add a breadcrumb link back to
easily let users get Edge back on the page.

See #1519

@stephenmcgruer stephenmcgruer marked this pull request as ready for review March 4, 2021 13:37
@stephenmcgruer stephenmcgruer reopened this Mar 4, 2021
stephenmcgruer added a commit that referenced this pull request Mar 9, 2021
This flag was used to fetch edge runs back when they were not tagged
with the experimental label. It is no longer useful, and is also
misleading (the name implies it might remove Edge from the default
query, which it does not do).  See
#2431 (comment)

This change also makes util/populate_dev_data.go closer to what is
shipped on wpt.fyi - the flags are updated and a new Edge experimental
test run is added for the local testing sha. The data for that run is a copy
of the Edge stable data for the same sha.

Finally, the `webdriver/` README.md file is extensively updated with more
description and a lot of debugging tips I've discovered over the last day!
After a lot of discussion, we have decided to remove Edge as a default
product from wpt.fyi due to overlap in browser engine between Chrome and
Edge. In a perfect world we would invest in per-user default products,
but there is no resourcing for that currently.

To try to minimize the disruption, we add a breadcrumb link back to
easily let users get Edge back on the page.

See #1519
@stephenmcgruer stephenmcgruer force-pushed the smcgruer/remove-edge-default branch from 53d3853 to 007e04f Compare March 26, 2021 17:47
@stephenmcgruer stephenmcgruer changed the title [WIP] Remove Edge from the default browser set Remove Edge as a default product, but add breadcrumb link Mar 26, 2021
Copy link
Collaborator

@KyleJu KyleJu left a comment

Choose a reason for hiding this comment

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

LGTM on the code. I will leave @foolip for the final stamp

@@ -386,6 +390,21 @@ class WPTApp extends PathInfo(WPTFlags(TestRunsUIBase)) {
return true;
}

showAddEdgeBackLink(queryParams) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also need to check if queryParams.run_id is true. If we are querying by run_id, we cannot simply add Edge to the UI

Copy link
Member

Choose a reason for hiding this comment

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

Another case where we can't add Edge back is the links for PR checks, such as https://wpt.fyi/?sha=ec5f766aec&label=pr_head. There is no Edge run to add back in that case.

The ideal behavior would be to detect the cases where the runs we are showing are inferred from the default product set, and where adding Edge to that default would still result in some runs to show. They might not be for the same commit in the case of aligned runs, but an additional run from Edge should appear.

Implementing this would presumably requiring using /api/runs a few times, so are there simpler approximations of this that get it right in most cases?

@foolip
Copy link
Member

foolip commented Mar 30, 2021

When loading https://smcgruer-remove-edge-d-dot-wptdashboard-staging.uk.r.appspot.com/ I still see Edge there. Are staging deployments not working?

@KyleJu
Copy link
Collaborator

KyleJu commented Mar 30, 2021

When loading https://smcgruer-remove-edge-d-dot-wptdashboard-staging.uk.r.appspot.com/ I still see Edge there. Are staging deployments not working?

Looks like its not deployed properly. Let me close and open it

@KyleJu
Copy link
Collaborator

KyleJu commented Mar 30, 2021

@foolip Should be working now!

@web-platform-tests web-platform-tests deleted a comment from fizietechD Mar 18, 2022
@gsnedders
Copy link
Member

What's the status here?

@foolip
Copy link
Member

foolip commented Apr 13, 2022

@KyleJu @past and I have talked about what to do about "default product set" when we add a front page.

In short, there will be a widget on the front page to get to results, and that widget will have Chrome, Firefox and Safari selected by default, but can be changed with a few clicks.

All URLs will specify the products to show, or redirect to URLs that do. I think we should redirect to the current default for existing product-less URLs.

A side effect of this will be that there won't be any short URLs that show what you (probably) want.

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.

6 participants