M1501: R​eport CSS errors to the devtools, both stored and live

[GauriGNaik]

Define a new trait called ParseErrorReporter in
components/style_traits/lib.rs with an appropriate method to report an error, and add a error_reporter member to ParserContext that uses this

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @larsbergstrom (or someone else) soon.

[jdm]

Looks good! We just need to address these style issues reported by ./mach test-tidy:

./components/style/parser.rs:21: Line is longer than 120 characters

./components/style/parser.rs:9: use statement is not in alphabetical order

    expected: style_traits::ParseErrorReporter

    found: stylesheets::Origin

./components/style/parser.rs:10: use statement is not in alphabetical order

    expected: stylesheets::Origin

    found: url::{Url, UrlParser}

./components/style/parser.rs:11: use statement is not in alphabetical order

    expected: url::{Url, UrlParser}

    found: style_traits::ParseErrorReporter

./components/style_traits/lib.rs:30: no newline at EOF

[jdm]

From test-tidy again: ./components/style/parser.rs:21: trailing whitespace
However, more importantly, these don't appear to compile. Did you build them locally?

[GauriGNaik]

To build successfully, it requires changes to be made to properties.mako.rs, selector_matching.rs, stylesheets.rs and viewport.rs. I have made changes to these files but still the build fails due to properties.rs and properties.rs gets generated on building servo.

I have tried but I have not been able to figure out what other changes I need to make in order to build servo successfully.

[GauriGNaik]

Once, we are clear on how to proceed we will resolve the checkstyle errors thrown by ./mach test-tidy in the next commit.

[jdm]

I propose changes that should make it easier to build correctly:

• store a Box<ParseErrorReporter + Send> value in the struct
• take Box<ParseErrorReporter + Send> arguments everywhere necessary
  There should be no need to add <'a> or &'a anywhere with these changes.

[jdm]

Please also use git rebase instead of merging changes from master, too. It results in much cleaner lists of commits.

[GauriGNaik]

Even after making the proposed changes, build fails because of properties.rs. Tried making changes to properties.rs according to the errors it throws. Managed to eliminate some of the errors. But it is getting too complicated to make changes to properties.rs. Could you please suggest some way out of this problem so that we can build servo successfully ? Thank you for your help.

[eefriedman]

Could you rebase and push what you currently have? It's hard to help without being able to read your current code.

[jdm]

According to the TravisCI link, many of the remaining errors are of the form "function expected 4 arguments, got 3" (which I presume has to do with need to pass the new error reporter value). If you correct that issue, I would expect the rest of the errors to disappear.

[GauriGNaik]

Yes correct, most of the errors are of that form and all the errors are in properties.rs. However, when I make those changes and do (./mach build --dev), initially I was able to reduce the number of errors. However, now when I build servo again, I lost the changes that I made to properties.rs and I am again back to the same number of errors I was facing earlier.

[eefriedman]

properties.rs is generated; you should be able to make equivalent changes to properties.mako.rs.

[jdm]

-S-awaiting-review +S-needs-squash
Great work! Please rebase these changes against master and squash them into a single commit so they can be merged!

---

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io

[jdm]

This can almost merge, but the changes to support/android-rs-glue from the rebase need to be reverted - specifically, that directory no longer exists on master. The build from TravisCI should be successful for this to merge correctly. You can commit changes using --amend and force push to avoid creating extra commits at this point.

[bors-servo]

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

[jdm]

Looks like something went wrong with the rebase, since there are merge conflicts now and changes are included in the commit that are already present in master. I'm going to go ahead and perform the surgery necessary to get this to land, in the interest of expediency :)

[jdm]

Rebased in #8682.