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

Define the HTTP Refresh header #2892

Merged
merged 3 commits into from Aug 9, 2017

Conversation

3 participants
@annevk

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2017

@domenic

domenic approved these changes Aug 8, 2017

Copy link
Member

left a comment

LGTM although as you note in web-platform-tests/wpt#6606 there are a lot of potential tests to be written.

source Outdated
headers.</p></li>

<li><p>Let <var>value</var> be the value of the header with each byte mapped to a code point
of equal value.</p></li>

This comment has been minimized.

Copy link
@domenic

domenic Aug 8, 2017

Member

It would be quite nice to test that this is the case and not, e.g., UTF-8 decoding.

This comment has been minimized.

Copy link
@annevk

annevk Aug 9, 2017

Author Member

I have confirmed it locally.

This comment has been minimized.

Copy link
@annevk

annevk Aug 9, 2017

Author Member

It's actually tested. Improved that test a bit.

@@ -115353,6 +115375,29 @@ interface <dfn>External</dfn> {
</dl>


<h3>`<dfn><code data-x="http-refresh">Refresh</code></dfn>`</h3>

This comment has been minimized.

Copy link
@domenic

domenic Aug 8, 2017

Member

I really thought we'd alphabetized the headers but I guess not...

This comment has been minimized.

Copy link
@annevk

annevk Aug 9, 2017

Author Member

Do you want that done as part of this PR/commit? Happy to sort that out.

This comment has been minimized.

Copy link
@domenic

domenic Aug 9, 2017

Member

Nah, we can leave it for later; I might be missing something too in how they're ordered.

source Outdated

<ol>
<li><p class="&#x0058;&#x0058;&#x0058;">Multiple `<code data-x="http-refresh">Refresh</code>`
headers.</p></li>

This comment has been minimized.

Copy link
@annevk

annevk Aug 9, 2017

Author Member

Shall I open an issue for this and point to that from here? Might be slightly better.

annevk added some commits Aug 9, 2017

@domenic

domenic approved these changes Aug 9, 2017

Copy link
Member

left a comment

Still LGTM

jgraham added a commit to web-platform-tests/wpt that referenced this pull request Aug 9, 2017

@annevk annevk merged commit 7e9f6b6 into master Aug 9, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@annevk annevk deleted the annevk/refresh branch Aug 9, 2017

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Aug 10, 2017

@annevk Do you know whether there are bugs tracking browser compliance here?

@annevk

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2017

@bzbarsky I didn't spot browser differences other than in the parser, which I think is always shared with <meta http-equiv=refresh> and I filed bugs for those (see #2844 (comment)). And I separately filed a bug against Firefox to decide on multiple header handling, which is also tracked by #2900. Did you notice something else?

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2017

No, that's it. Thank you!

@whatwg whatwg deleted a comment Aug 13, 2017

rachelandrew added a commit to rachelandrew/web-platform-tests that referenced this pull request Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.