-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Iframes with missing redirect location should fire load event and queue RT entry #33317
Conversation
t.add_cleanup(() => frame.remove()); | ||
await new Promise(resolve => { | ||
frame.addEventListener('load', resolve); | ||
frame.src = 'resources/bad-redirect.py'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe rename to redirect-without-location.py or add a comment as to why this redirect is bad?
document.body.appendChild(frame); | ||
}); | ||
|
||
const entries = performance.getEntriesByName(frame.src); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? I thought getEntriesByName
needs the absolute URL..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! The src
attribute returns the full URL
<script src="/resources/testharnessreport.js"></script> | ||
<body> | ||
<script> | ||
promise_test(async t => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level note - would be nice to use the resource-loaders.js
infra for this. Would also be nice to test (if not tested already elsewhere) that it's only the initial src that's exposed and not the redirected one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter is tested elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ue RT entry (web-platform-tests#33317) * Iframes with missing redirect location should fire load event and queue RT entry See whatwg/html#7531 * Rename and use loader
Iframes with missing redirect location (e.g. 302 response code and no
location
header) should fire load event and queue RT entry