Skip to content

wpt: Delete viewport-apply-initial-scale-after-navigation.html#43701

Merged
xiaochengh merged 1 commit intoservo:mainfrom
shubhamg13:view_nav
Apr 1, 2026
Merged

wpt: Delete viewport-apply-initial-scale-after-navigation.html#43701
xiaochengh merged 1 commit intoservo:mainfrom
shubhamg13:view_nav

Conversation

@shubhamg13
Copy link
Copy Markdown
Member

@shubhamg13 shubhamg13 commented Mar 27, 2026 •
edited
Loading

Scenario proposed should be best tested using unit tests.

Testing: TODO (Waiting for #43160)
Fixes: #41582

@shubhamg13
Delete Tests
1df6732 
Signed-off-by: Shubham Gupta <[email protected]>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 27, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Mar 27, 2026

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#58800) with upstreamable changes.

@servo-wpt-sync servo-wpt-sync mentioned this pull request Mar 27, 2026
Merged
stevennovaryo
stevennovaryo approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@stevennovaryo stevennovaryo left a comment

Choose a reason for hiding this comment

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

After closer inspection of viewport meta tag, these WPTs wouldn't be relevant anymore. Being enabled only in mobile devices it should be tested with unit test.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 27, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

servo-wpt-sync commented Mar 27, 2026

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#58800) title and body.

@xiaochengh
Copy link
Copy Markdown
Contributor

xiaochengh commented Mar 30, 2026

I'm not following this change.

  • What's the reason for the flaky timeout?
  • For mobile-only tests, is there an existing guideline as reference? Simply removing from WPT seems not right, as there should be other mobile-only tests in WPT (IIRC)

@shubhamg13
Copy link
Copy Markdown
Member Author

shubhamg13 commented Mar 30, 2026

  • For mobile-only tests, is there an existing guideline as reference? Simply removing from WPT seems not right, as there should be other mobile-only tests in WPT (IIRC)

There is an old pending open issue for referencing this: web-platform-tests/wpt#11717

@xiaochengh
Copy link
Copy Markdown
Contributor

xiaochengh commented Mar 30, 2026

  • For mobile-only tests, is there an existing guideline as reference? Simply removing from WPT seems not right, as there should be other mobile-only tests in WPT (IIRC)

There is an old pending open issue for referencing this: web-platform-tests/wpt#11717

Yeah, but that issue has not been updated for years. What's the current status regarding mobile-only WPT?

@shubhamg13
Copy link
Copy Markdown
Member Author

shubhamg13 commented Mar 30, 2026

Yeah, but that issue has not been updated for years. What's the current status regarding mobile-only WPT?

Same set of tests are being run on Chrome Android. And they are curated in such a way that even if mobile feature is disabled on windows, they will fallback to default value. So, we don't know whether something is wrong.

Apart from that there are some manual tests.

@shubhamg13
Copy link
Copy Markdown
Member Author

shubhamg13 commented Mar 30, 2026

  • What's the reason for the flaky timeout?

We start a test, and navigate to another page aka inner page. And inner page has no reference about what to compare and it TIMEOUTs.

@xiaochengh
Copy link
Copy Markdown
Contributor

xiaochengh commented Apr 1, 2026

  • What's the reason for the flaky timeout?

We start a test, and navigate to another page aka inner page. And inner page has no reference about what to compare and it TIMEOUTs.

Hmm, this is an OK reason to remove the test as it's not well written.

@xiaochengh xiaochengh added this pull request to the merge queue Apr 1, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 1, 2026
Merged via the queue into servo:main with commit 23e69b0 Apr 1, 2026
34 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 1, 2026
@shubhamg13 shubhamg13 deleted the view_nav branch April 1, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiaochengh xiaochengh xiaochengh approved these changes

@mrobinson mrobinson Awaiting requested review from mrobinson

@jdm jdm Awaiting requested review from jdm

+1 more reviewer

@stevennovaryo stevennovaryo stevennovaryo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Intermittent PASS in /visual-viewport/viewport-apply-initial-scale-after-navigation.html

5 participants

@shubhamg13 @servo-wpt-sync @xiaochengh @stevennovaryo @servo-highfive