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

Various fixes to make github less error-prone #7719

Merged
merged 4 commits into from Sep 24, 2015
Merged

Conversation

@Manishearth
Copy link
Member

Manishearth commented Sep 23, 2015

r? @Ms2ger

Review on Reviewable

@highfive
Copy link

highfive commented Sep 23, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@Manishearth
Copy link
Member Author

Manishearth commented Sep 23, 2015

I run this with the userscript:

window.history = {};
CanvasRenderingContext2D.prototype.fillText = function(a,b,c) {alert(a);alert(b);alert(c);}
document.domain="";
dqs = Document.prototype.querySelector;
Document.prototype.querySelector = function(a) {
    alert("doc.qS: "+a);
    if (a == ':target') {
        return null;
    }
    ret = dqs.apply(this, [a]);
    alert("doc.qS over");
    return ret;
}
delete window.WebSocket;

WebSocket is broken because we coerce [] in its constructor to "", which then causes a SyntaxError. querySelector is broken because we don't support :target, though I'm working on that.

window.history isn't easy to fix now since we don't really have the browsing context framework. I haven't yet looked at document.domain or filltext().

We also need support for form.elements

@bors-servo
Copy link
Contributor

bors-servo commented Sep 24, 2015

The latest upstream changes (presumably #7710) made this pull request unmergeable. Please resolve the merge conflicts.

@Manishearth Manishearth force-pushed the Manishearth:ghfix branch from b874704 to e186772 Sep 24, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 24, 2015

Please squash into logical commits that pass tests for review.

@Manishearth Manishearth force-pushed the Manishearth:ghfix branch from e186772 to 982e55a Sep 24, 2015
@Manishearth
Copy link
Member Author

Manishearth commented Sep 24, 2015

Done

@nox
Copy link
Member

nox commented Sep 24, 2015

Reviewed 1 of 2 files at r2, 8 of 8 files at r3.
Review status: 9 of 11 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


tests/wpt/metadata/html/semantics/forms/form-control-infrastructure/form.html.ini, line 3 [r3] (raw file):
Please file a follow-up issue for those that remain.


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 24, 2015

@bors-servo r+

Do file that issue.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 24, 2015

📌 Commit 982e55a has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Sep 24, 2015

Testing commit 982e55a with merge 2623f58...

bors-servo pushed a commit that referenced this pull request Sep 24, 2015
Various fixes to make github less error-prone

r? @Ms2ger

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

bors-servo commented Sep 24, 2015

@bors-servo bors-servo merged commit 982e55a into servo:master Sep 24, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Manishearth Manishearth deleted the Manishearth:ghfix branch Feb 6, 2017
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

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