Skip to content

/ servo

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

Implement origin concept and browsing contextless documents. #8658

Merged
merged 3 commits into from Apr 13, 2016
Merged

Implement origin concept and browsing contextless documents. #8658

merged 3 commits into from Apr 13, 2016

Conversation

@jdm
Copy link
Member

jdm commented Nov 23, 2015

These pave the way for implementing other parts of specifications more thoroughly.

Review on Reviewable
@jdm
Copy link
Member Author

jdm commented Nov 23, 2015

@bors-servo: try
@bors-servo
Copy link
Contributor

bors-servo commented Nov 23, 2015

⌛ Trying commit f3fd51c with merge 2188c3c...
bors-servo added a commit that referenced this pull request Nov 23, 2015
@bors-servo
Auto merge of #8658 - jdm:origin2, r=<try>
2188c3c 
Implement origin concept and browsing contextless documents.

These pave the way for implementing other parts of specifications more thoroughly.
@jdm jdm force-pushed the jdm:origin2 branch from f3fd51c to d47fed4 Nov 23, 2015
@eefriedman
Copy link
Contributor

eefriedman commented Nov 23, 2015

The travis build failed... but it looks like it's not related to this PR; somehow travis is doing manifest checking, but homu isn't?
@eefriedman eefriedman self-assigned this Nov 23, 2015
@jdm
Copy link
Member Author

jdm commented Nov 23, 2015

Yeah, we haven't updated the builders since servo/saltfs#163 merged.
@jdm
Copy link
Member Author

jdm commented Nov 23, 2015

It is related to this PR; I need to run cargo-update again to get rid of a dependency that was added while in development.
@eefriedman
Copy link
Contributor

eefriedman commented Nov 23, 2015

Reviewed 7 of 7 files at r1, 8 of 8 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

components/script/dom/document.rs, line 1446 [r2] (raw file):
Is the meaning of "served over the network" actually defined anywhere in the spec? If not, please file a spec bug. ("Has a browsing context" seems like a reasonable approximation for now.)

components/script/origin.rs, line 9 [r2] (raw file):
You might want to link to the definition in the HTML spec? It's not quite the same thing given the inclusion of aliases.

components/script/origin.rs, line 19 [r2] (raw file):
This representation of an "alias" isn't really workable; an alias resolves to the origin of another document even if that origin changes.

components/script/origin.rs, line 35 [r2] (raw file):
It's probably not a good idea to use the file: protocol here; file:///tmp isn't a valid URL on Windows.

components/script/origin.rs, line 71 [r2] (raw file):
It might be better to force callers to explicitly resolve an Origin to a URLOrigin? Not sure.

Comments from the review on Reviewable.io
@jdm jdm removed the S-awaiting-review label Nov 23, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Nov 25, 2015

☔ The latest upstream changes (presumably #8667) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm jdm mentioned this pull request Nov 30, 2015
Closed
@KiChjang KiChjang mentioned this pull request Dec 15, 2015
Closed
@jdm jdm force-pushed the jdm:origin2 branch from d47fed4 to 209c41f Jan 4, 2016
@jdm
Copy link
Member Author

jdm commented Jan 4, 2016

I've only rebased; I haven't addressed any previous comments.
@jdm jdm force-pushed the jdm:origin2 branch from 209c41f to 7a15ac3 Jan 4, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Jan 10, 2016

☔ The latest upstream changes (presumably #9201) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm
Copy link
Member Author

jdm commented Apr 12, 2016

This looks good to me!
@jdm jdm removed the S-awaiting-review label Apr 12, 2016
@jdm @Ms2ger
Implement origin concept.
a8233a1
@Ms2ger Ms2ger force-pushed the jdm:origin2 branch from 1d5a6b8 to 315c83e Apr 13, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 13, 2016

@bors-servo r=Ms2ger+jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

📌 Commit 315c83e has been approved by Ms2ger+jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

⌛ Testing commit 315c83e with merge e45440b...
bors-servo added a commit that referenced this pull request Apr 13, 2016
@bors-servo
Auto merge of #8658 - jdm:origin2, r=Ms2ger+jdm
Loading status checks…
e45440b 
Implement origin concept and browsing contextless documents.

These pave the way for implementing other parts of specifications more thoroughly.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8658)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

💔 Test failed - mac-rel-wpt
jdm added 2 commits Apr 13, 2016
@jdm @Ms2ger
Use origin for document.domain.
fa42b45
@jdm @Ms2ger
Use origin when manipulating cookies.
Loading status checks…
90454c2
@Ms2ger Ms2ger force-pushed the jdm:origin2 branch from 315c83e to 90454c2 Apr 13, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Apr 13, 2016

@bors-servo r=Ms2ger+jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

📌 Commit 90454c2 has been approved by Ms2ger+jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

⌛ Testing commit 90454c2 with merge 9b57d8d...
bors-servo added a commit that referenced this pull request Apr 13, 2016
@bors-servo
Auto merge of #8658 - jdm:origin2, r=Ms2ger+jdm
Loading status checks…
9b57d8d 
Implement origin concept and browsing contextless documents.

These pave the way for implementing other parts of specifications more thoroughly.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8658)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 13, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 90454c2 into servo:master Apr 13, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the jdm:origin2 branch Apr 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@jdm @bors-servo @eefriedman @KiChjang @emilio @Ms2ger @frewsxcv @highfive
You can’t perform that action at this time.