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

Implements document.lastModified for #2972 #3127

Closed
wants to merge 8 commits into from
Closed

Conversation

@ghost
Copy link

ghost commented Aug 20, 2014

No description provided.

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Aug 20, 2014

Critic review: https://critic.hoppipolla.co.uk/r/2414

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@jdm
Copy link
Member

jdm commented Aug 26, 2014

Thanks @mechaxl! I've left some comments on Critic that should be addressed.

mechaxl added 2 commits Aug 27, 2014
@jdm
Copy link
Member

jdm commented Aug 28, 2014

Summary
=======

Ran 3788 tests (364 parents, 3424 subtests)
Expected results: 3784
Unexpected results: 4 (FAIL: 1, PASS: 3)

Unexpected Results
==================

/html/dom/documents/resource-metadata-management/document-lastModified.html
---------------------------------------------------------------------------
PASS expected FAIL lastModified should return the last modified date and time

/html/dom/documents/resource-metadata-management/document-lastModified-01.html
------------------------------------------------------------------------------
PASS expected FAIL Date returned by lastModified is in the user's local time zone.
PASS expected FAIL Date returned by lastModified is in the form "MM/DD/YYYY hh:mm:ss".
FAIL Date returned by lastModified is current.
@jdm
Copy link
Member

jdm commented Aug 28, 2014

The ini file for document-lastModified will need to be updated to account for the updated test results.

@ghost
Copy link
Author

ghost commented Aug 28, 2014

@jdm, where is the ini file located at?

mechaxl added 2 commits Aug 28, 2014
Pulling in upstream changes
Ms2ger added a commit to Ms2ger/servo that referenced this pull request Sep 3, 2014
…d if not set by the Last-Modified header
@jdm
Copy link
Member

jdm commented Sep 4, 2014

Great! Let's wait for Travis to go green, and then I'd like to squash everything together into a single commit.

Ms2ger added a commit to Ms2ger/servo that referenced this pull request Sep 4, 2014
Ms2ger added a commit that referenced this pull request Sep 4, 2014
Implement document.lastModified (fixes #2972, #3127); r=jdm+Manishearth
@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 4, 2014

@mechaxl, thanks a lot! I squashed your commits and landed them over in #3204.

@jdm
Copy link
Member

jdm commented Sep 4, 2014

In that case, let's close this PR.

@jdm jdm closed this Sep 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.