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

Update parser state only after rule is successfully parsed #18139

Merged
merged 1 commit into from Aug 21, 2017

Conversation

@upsuper
Copy link
Member

upsuper commented Aug 18, 2017

This is expected to fix several web-platform tests mentioned in bug 1389187.


This change is Reviewable

@highfive
Copy link

highfive commented Aug 18, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/stylesheets/rule_parser.rs
  • @canaltinova: components/style/stylesheets/rule_parser.rs
  • @emilio: components/style/stylesheets/rule_parser.rs
@highfive
Copy link

highfive commented Aug 18, 2017

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@upsuper
Copy link
Member Author

upsuper commented Aug 18, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2017

Trying commit 66d4834 with merge 1761120...

bors-servo added a commit that referenced this pull request Aug 18, 2017
Update parser state only after rule is successfully parsed

This is expected to fix several web-platform tests mentioned in [bug 1389187](https://bugzilla.mozilla.org/show_bug.cgi?id=1389187).
@upsuper
Copy link
Member Author

upsuper commented Aug 18, 2017

@highfive highfive assigned SimonSapin and unassigned mbrubeck Aug 18, 2017
@upsuper
Copy link
Member Author

upsuper commented Aug 18, 2017

(try should fail with unexpected passes, I suppose)

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2017

💔 Test failed - linux-rel-css

@upsuper upsuper force-pushed the upsuper-forks:parser-state branch from 66d4834 to f0fdf76 Aug 18, 2017
@upsuper
Copy link
Member Author

upsuper commented Aug 18, 2017

This seems to cause assertion on css-namespaces-3/syntax-014.xml on Gecko. Will investigate. Please don't r+ for now.

@upsuper
Copy link
Member Author

upsuper commented Aug 18, 2017

Hmmm... so the problem here is that we need to freeze the namespaces table when we start parsing rules other than @namespace, however, if the parsing failed, we are supposed to be able to accept another @namespace rule, but we have froze the table before.

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2017

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

@SimonSapin
Copy link
Member

SimonSapin commented Aug 19, 2017

Emilio said #18142 should help with that. But it also conflicts, so this needs some more changes.

@upsuper upsuper force-pushed the upsuper-forks:parser-state branch from f0fdf76 to 6fef38e Aug 20, 2017
@upsuper upsuper force-pushed the upsuper-forks:parser-state branch from 6fef38e to c15ae3b Aug 20, 2017
@upsuper
Copy link
Member Author

upsuper commented Aug 20, 2017

@SimonSapin
Copy link
Member

SimonSapin commented Aug 21, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Aug 21, 2017

📌 Commit c15ae3b has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Aug 21, 2017

Testing commit c15ae3b with merge 430b171...

bors-servo added a commit that referenced this pull request Aug 21, 2017
Update parser state only after rule is successfully parsed

This is expected to fix several web-platform tests mentioned in [bug 1389187](https://bugzilla.mozilla.org/show_bug.cgi?id=1389187).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18139)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 21, 2017

@bors-servo bors-servo merged commit c15ae3b into servo:master Aug 21, 2017
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
@upsuper upsuper deleted the upsuper-forks:parser-state branch Aug 21, 2017
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 21, 2017
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 21, 2017
weilonge pushed a commit to fx-dev-playground/gecko that referenced this pull request Aug 22, 2017
weilonge pushed a commit to fx-dev-playground/gecko that referenced this pull request Aug 22, 2017
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Aug 24, 2017
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Aug 24, 2017
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: F8wa1hxNtCZ

UltraBlame original commit: 39e9a20aacfcde749c386b048a48d86144a9f6fa
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: 7q1ITUpRcS2

UltraBlame original commit: 02095185a9479e03f31f8a38d05eb64cd9516bf2
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: F8wa1hxNtCZ

UltraBlame original commit: 39e9a20aacfcde749c386b048a48d86144a9f6fa
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: 7q1ITUpRcS2

UltraBlame original commit: 02095185a9479e03f31f8a38d05eb64cd9516bf2
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: F8wa1hxNtCZ

UltraBlame original commit: 39e9a20aacfcde749c386b048a48d86144a9f6fa
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
MozReview-Commit-ID: 7q1ITUpRcS2

UltraBlame original commit: 02095185a9479e03f31f8a38d05eb64cd9516bf2
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.