Expose nested browsing context status in RequestClient#41661
Expose nested browsing context status in RequestClient#41661TimvdLippe merged 4 commits intoservo:mainfrom
Conversation
|
🔨 Triggering try run (#20692309109) for Linux (WPT) |
|
Test results for linux-wpt from try job (#20692309109): Flaky unexpected result (33)
Stable unexpected results that are known to be intermittent (29)
|
|
✨ Try run (#20692309109) succeeded. |
bd70d55 to
8cdc59c
Compare
|
There seems to be no tests covering this. Maybe someone else can suggest something. |
|
From the issue:
@WaterWhisperer Did you debug these tests to see why they are still not running as expected? Do we need to implement a different feature as well to make them pass? If you are unsure how to debug such tests, feel free to start a thread on Zulip so we can help you out. |
Thanks for the feedback! I've investigated this further: After looking at the test details, I guess that the |
components/net/fetch/methods.rs
Outdated
There was a problem hiding this comment.
Can you maybe also implement step 3?
Let upgrade state be the result of executing § 4.2 Should insecure requests be upgraded for client? upon request’s client.
For that you need to modify the request client as well and pass the relevant information from the document (skip detached clients for now): https://w3c.github.io/webappsec-upgrade-insecure-requests/#insecure-requests-policy
That should be sufficient for this PR. If you are eager, you can then also tackle CSP integration, which the tests appear to be using: https://w3c.github.io/webappsec-upgrade-insecure-requests/#delivery Once you implement that header, I expect the tests to start passing. For that, we have a fork of the CSP crate that we need to update as well: https://github.com/servo/rust-content-security-policy/tree/servo-csp
There was a problem hiding this comment.
Ah we already implement it on Document:
servo/components/script/dom/document.rs
Line 3768 in de27dc6
There was a problem hiding this comment.
Thanks! I'll start to implement it.
|
Btw there is conflict. |
Thanks, I'll rebase it |
Signed-off-by: WaterWhisperer <[email protected]>
8cdc59c to
26f9df2
Compare
|
I'm not sure if there's something wrong with my implementation, and running the relevant tests locally not only makes the timed out tests non-timeout, but also causes the tests that otherwise passed to fail :( |
|
🔨 Triggering try run (#20696142344) for Linux (WPT) |
|
Let's run CI so we can analyze the results and help you further. Thanks for tackling! |
|
Test results for linux-wpt from try job (#20696142344): Flaky unexpected result (38)
Stable unexpected results that are known to be intermittent (26)
Stable unexpected results (13)
|
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
Actually I bet one problem is this: servo/components/script/navigation.rs Line 197 in 8295a98 We're not setting the request client for navigation requests. |
Thanks, I tried setting the RequestClient for navigation requests in |
Signed-off-by: WaterWhisperer <[email protected]>
|
is missing a client, and so is servo/components/script/dom/websocket.rs Line 281 in c68748a |
Signed-off-by: WaterWhisperer <[email protected]>
|
🔨 Triggering try run (#20719997808) for Linux (WPT) |
|
Test results for linux-wpt from try job (#20719997808): Flaky unexpected result (37)
Stable unexpected results that are known to be intermittent (25)
Stable unexpected results (2)
|
|
|
Signed-off-by: WaterWhisperer <[email protected]>
TimvdLippe
left a comment
There was a problem hiding this comment.
Nice work and thanks for tackling the additional missing parts!
I've tried to highlight the most common strategies that I use to narrow down the cause of failures in WPT tests. This is useful for cases like servo/servo#41661 where the expected test result changes did not materialize and we ask contributors to help diagnose why. --------- Signed-off-by: Josh Matthews <[email protected]> Co-authored-by: Mukilan Thiyagarajan <[email protected]> Co-authored-by: Martin Robinson <[email protected]>
implement step 2.2 of https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request by adding an is_nested_browsing_context field to RequestClient
Testing:
./mach test-wpt tests/wpt/tests/upgrade-insecure-requestsand./mach test-wpt tests/wpt/tests/mixed-contentFixes: #41639