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.

Sign up for GitHub

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

Jump to bottom

Return None as style for elements not in a document #6851

Merged
merged 1 commit into from
Jul 30, 2015
Merged

Return None as style for elements not in a document #6851

merged 1 commit into from
Jul 30, 2015

Conversation

dzbarsky
Copy link
Contributor

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 30, 2015
@pcwalton
Copy link
Contributor

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit 36145d0 has been approved by pcwalton

@jdm
Copy link
Member

jdm commented Jul 30, 2015

@bors-servo: r-
I'm not convinced that we've decided what to do here, given the IRC conversation. This is web-compatibility land.

@jdm
Copy link
Member

jdm commented Jul 30, 2015

Relevant log: http://logs.glob.uno/?c=mozilla%23servo#c248309

@dzbarsky
Copy link
Contributor Author

For this testcase, we will now report "". This matches Chrome and Safari, so it should be web-compatible.

var foo = document.createElement('div');
foo.style.setPropertyValue('left', '15px');
alert(getComputedStyle(foo).getPropertyValue('left'));

@pcwalton
Copy link
Contributor

Can we move forward here? This is causing a lot of crashes and is blocking people's work.

@jdm jdm added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 30, 2015
@jdm
Copy link
Member

jdm commented Jul 30, 2015

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit 36145d0 has been approved by jdm

@jdm
Copy link
Member

jdm commented Jul 30, 2015

Filed #6860 about the uncertainty here.

@bors-servo
Copy link
Contributor

⌛ Testing commit 36145d0 with merge e42fddd...

bors-servo pushed a commit that referenced this pull request Jul 30, 2015
Auto merge of #6851 - dzbarsky:gCS, r=jdm
e42fddd 
Return None as style for elements not in a document



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

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 36145d0 into servo:master Jul 30, 2015
@dzbarsky dzbarsky deleted the gCS branch July 31, 2015 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dzbarsky @pcwalton @bors-servo @jdm @highfive