Skip to content

Pass rule start to parser rather than just a location. #277

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

Merged
merged 2 commits into from
Dec 4, 2020
Merged

Conversation

emilio
Copy link
Member

@emilio emilio commented Dec 3, 2020

This is:

  • More generic, as the state has more information like the source
    index.

  • Slightly cheaper to compute, as the source location requires math to
    compute, which the parser state doesn't

I need this to improve CSS sanitization in Gecko.

https://bugzilla.mozilla.org/show_bug.cgi?id=1680084

@emilio
Copy link
Member Author

emilio commented Dec 3, 2020

r? @SimonSapin / @upsuper / @heycam

This is:

 * More generic, as the state has more information like the source
   index.

 * Slightly cheaper to compute, as the source location requires math to
   compute, which the parser state doesn't

I need this to improve CSS sanitization in Gecko.

https://bugzilla.mozilla.org/show_bug.cgi?id=1680084
@heycam
Copy link
Contributor

heycam commented Dec 4, 2020

Should we have a unit test for the fixed at-rule location?

@emilio
Copy link
Member Author

emilio commented Dec 4, 2020

We have one integration test in Gecko for it and no existing tests for SourceLocation stuff, so it's not really trivial. I looked into adding one and it requires a bunch of boilerplate so not super-exciting...

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

📌 Commit 1e4dbb7 has been approved by heycam

@bors-servo
Copy link
Contributor

⌛ Testing commit 1e4dbb7 with merge a37566b...

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis
Approved by: heycam
Pushing a37566b to master...

@bors-servo bors-servo merged commit a37566b into master Dec 4, 2020
@emilio emilio deleted the start branch December 4, 2020 03:10
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 4, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 5, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 6, 2020
…eycam, a=RyanVM

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 6, 2020
…eycam, a=RyanVM

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this pull request Dec 10, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this pull request Dec 10, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 14, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677

UltraBlame original commit: 7fbd27be335d24b67954e6b44701f9ff1cd117cd
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 14, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677

UltraBlame original commit: 7fbd27be335d24b67954e6b44701f9ff1cd117cd
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 14, 2020
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677

UltraBlame original commit: 7fbd27be335d24b67954e6b44701f9ff1cd117cd
emilio added a commit to servo/servo that referenced this pull request Feb 26, 2021
The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
aosmond pushed a commit to aosmond/gecko that referenced this pull request Nov 27, 2021
…eycam, a=RyanVM

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
Mossop pushed a commit to Mossop/firefox-working that referenced this pull request Apr 30, 2025
…eycam, a=RyanVM

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…eycam

The changes should be trivial.

The third_party changes are up for review in
servo/rust-cssparser#277 (and of course I'll
land with a bump to 0.28 rather than the override after that gets r+'d).

The basic idea is that with this we have the actual start offset of the
rule, so we wouldn't include html comments or other invalid stuff we
discard during sanitization in bug 1680084. But that's a separate
change.

Differential Revision: https://phabricator.services.mozilla.com/D98677
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants