Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSupport line number offsets for inline stylesheets #15693
Comments
|
On the other hand, we could avoid touching the CSS parser at all and store the offset in ParserContext, and pass it as an argument to the report_error API instead. That's an easier plan. |
|
Claiming this one for a contributor. |
|
I am working on it. |
|
@PratikDhanave Have you made any progress? Any questions? |
|
No not yet
…On Thu, Mar 16, 2017 at 1:29 PM, Josh Matthews ***@***.***> wrote:
@PratikDhanave <https://github.com/PratikDhanave> Have you made any
progress? Any questions?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15693 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACFKH_FHhVl2FyazMguasUjr0ng1nJrbks5rmOvdgaJpZM4MI9KP>
.
|
|
Unassigning due to lack of activity. |
|
Necessary work:
Code: |
|
The idea here is that we get incorrect line numbers in HTML like this:
<style>
:matches(thead)[align=absmiddle i] {
color: red
}
</style>If we run
when the invalid CSS is clearly not line 2 of the actual file. |
|
Maybe also a column offset, for the first line? Especially for |
|
I think that will require additional changes to html5ever. Let's keep this focused on line offsets for now. |
|
Also counting "columns" is more tricky than it seems. UTF-8 bytes v.s. code points v.s. grapheme clusters v.s. east-asian width… Deferring for now avoids this question. |
|
@highfive: assign me |
|
Hey @mckaymatt! Thanks for your interest in working on this issue. It's now assigned to you! |
|
@jdm when you say
Do you mean |
|
Yes. |
…nSapin Support line number offsets for inline stylesheets <!-- Please describe your changes on the following line: --> This allows accurate line numbers when reporting stylesheet errors. @jdm This is going to require some effort to merge my changes with other recent changes to `ParserContext`. Because of that I would appreciate a quick sanity check before I put the time into performing the merge. For example, should I store the `offset` as a u64, or should it be an Option? --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15693 (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/16394) <!-- Reviewable:end -->
…nSapin Support line number offsets for inline stylesheets <!-- Please describe your changes on the following line: --> This allows accurate line numbers when reporting stylesheet errors. @jdm This is going to require some effort to merge my changes with other recent changes to `ParserContext`. Because of that I would appreciate a quick sanity check before I put the time into performing the merge. For example, should I store the `offset` as a u64, or should it be an Option? --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15693 (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/16394) <!-- Reviewable:end -->
The DOM-side work will be very similar to the work in #14963.