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

Panic if DOMTokenList#contains is called for an unparsed attribute. #4084

Closed
wants to merge 1 commit into from
Closed

Panic if DOMTokenList#contains is called for an unparsed attribute. #4084

wants to merge 1 commit into from

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 24, 2014

Previously, if the attribute was not parsed into a token list, and the
tokens() method returned None, DOMTokenList#contains would silently return
false. This issue was encountered in
#4076 and took quite some time to
figure out.
@Ms2ger
Panic if DOMTokenList#contains is called for an unparsed attribute.
Loading status checks…
449aaec 
Previously, if the attribute was not parsed into a token list, and the
tokens() method returned None, DOMTokenList#contains would silently return
false. This issue was encountered in
<#4076> and took quite some time to
figure out.
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Nov 24, 2014

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

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 Nov 24, 2014

Want to review this, @saneyuki?
@tetsuharuohzeki
Copy link
Member

tetsuharuohzeki commented Nov 24, 2014

ok. I reviewed.
@jdm

This comment has been minimized.

Sign in to view
Copy link

jdm commented on 449aaec Nov 24, 2014

r+
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented on 449aaec Nov 24, 2014

saw approval from jdm
at Ms2ger@449aaec

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 24, 2014

merging Ms2ger/servo/tokenlist-robust = 449aaec into auto

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 24, 2014

Ms2ger/servo/tokenlist-robust = 449aaec merged ok, testing candidate = 51aa2fd

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 24, 2014

all tests pass:
success: http://build.servo.org/builders/linux1/builds/51
success: http://build.servo.org/builders/linux2/builds/51
success: http://build.servo.org/builders/android/builds/118
success: http://build.servo.org/builders/mac1/builds/268
success: http://build.servo.org/builders/mac2/builds/268

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Nov 24, 2014

fast-forwarding master to auto = 51aa2fd
bors-servo pushed a commit that referenced this pull request Nov 24, 2014
bors-servo
auto merge of #4084 : Ms2ger/servo/tokenlist-robust, r=jdm
Loading status checks…
51aa2fd 
Previously, if the attribute was not parsed into a token list, and the
tokens() method returned None, DOMTokenList#contains would silently return
false. This issue was encountered in
<#4076> and took quite some time to
figure out.
@bors-servo bors-servo closed this Nov 24, 2014
@Ms2ger Ms2ger deleted the Ms2ger:tokenlist-robust branch Dec 5, 2014
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
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Ms2ger @hoppipolla-critic-bot @jdm @tetsuharuohzeki @bors-servo
You can’t perform that action at this time.