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 upCSSRule, CSSRuleList, CSSStyleSheet partial implementation #10765
Conversation
highfive
commented
Apr 20, 2016
|
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. |
highfive
commented
Apr 20, 2016
|
Heads up! This PR modifies the following files:
|
highfive
commented
Apr 20, 2016
|
Next steps:
Basically, our goal for this project is to create the correct associations between the various DOM reflection types (Stylesheet/CSSStyleSheet, CSSRule, CSSRuleList, StyleSheetList) and the underlying data (Stylesheet, CSSRule). Does that make sense? -S-awaiting-review +S-needs-code-changes Reviewed 8 of 8 files at r1. components/script/dom/cssrule.rs, line 26 [r1] (raw file): components/script/dom/cssrule.rs, line 27 [r1] (raw file): components/script/dom/cssrule.rs, line 32 [r1] (raw file): components/script/dom/cssrule.rs, line 52 [r1] (raw file): components/script/dom/cssrulelist.rs, line 11 [r1] (raw file): components/script/dom/cssrulelist.rs, line 18 [r1] (raw file): components/script/dom/cssstylesheet.rs, line 15 [r1] (raw file): components/script/dom/cssstylesheet.rs, line 16 [r1] (raw file): components/script/dom/cssstylesheet.rs, line 45 [r1] (raw file): components/script/dom/document.rs, line 1817 [r1] (raw file): components/script/dom/webidls/CSSRule.webidl, line 9 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): home/ibobra/Documents/ServoFork/servo/components/script/dom/cssrule.rs:19:11: 19:31 error: wrong number of type arguments: expected 1, found 0 [E0243] Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 52 [r1] (raw file): fn Type_(&self) -> u16 {
} Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): components/script/dom/cssrule.rs, line 52 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): Compiling script v0.0.1 (file:///home/ibobra/Documents/ServoFork/servo/components/script) Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): I dont understand what would be the appropriate "no_jsmanaged_fields" entry.. tried a few like CSSRUle, ServoSelectorImpl etc. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): so as per your directions we added: no_jsmanaged_fields!(ServoSelectorImpl); But still we are stuck with the following error: error: the trait bound Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 26 [r1] (raw file): impl JSTraceable for CSSRule<ServoSelectorImpl> {
fn trace(&self, _: *mut JSTracer) {}
}Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 52 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed. components/script/dom/cssrule.rs, line 52 [r1] (raw file): Comments from Reviewable |
|
Reviewed 1 of 8 files at r1. components/script/dom/document.rs, line 1817 [r1] (raw file): We have finally written the function as
What should be given as the 3rd parameter in "CSSStyleSheet::new". I have marked it with a "?".
Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed. components/script/dom/webidls/CSSRule.webidl, line 9 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed. components/script/dom/document.rs, line 1817 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed. components/script/dom/cssstylesheet.rs, line 16 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed. components/script/dom/cssstylesheet.rs, line 15 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed. components/script/dom/cssstylesheet.rs, line 15 [r1] (raw file): components/script/dom/cssstylesheet.rs, line 16 [r1] (raw file): Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed. components/script/dom/cssstylesheet.rs, line 16 [r1] (raw file): Comments from Reviewable |
|
Getting better :) Reviewed 1 of 1 files at r12, 3 of 3 files at r13, 1 of 1 files at r14, 5 of 5 files at r15, 4 of 4 files at r16, 1 of 1 files at r17, 2 of 2 files at r18, 5 of 5 files at r19, 13 of 13 files at r20, 10 of 10 files at r21, 13 of 13 files at r22, 5 of 5 files at r23, 5 of 5 files at r24, 3 of 3 files at r25, 5 of 5 files at r26, 5 of 5 files at r27, 6 of 6 files at r28, 4 of 4 files at r29, 6 of 6 files at r30, 2 of 2 files at r31. components/script/dom/cssstylerule.rs, line 26 [r30] (raw file): components/script/dom/cssstylerule.rs, line 32 [r30] (raw file): components/script/dom/cssstylesheet.rs, line 29 [r30] (raw file): components/script/dom/cssstylesheet.rs, line 50 [r30] (raw file): components/script/dom/document.rs, line 1832 [r30] (raw file): components/script/dom/document.rs, line 1834 [r30] (raw file): components/script/dom/stylesheet.rs, line 8 [r30] (raw file): components/script/dom/stylesheet.rs, line 39 [r30] (raw file): Comments from Reviewable |
|
@bors-servo: try |
CSSRule, CSSRuleList, CSSStyleSheet partial implementation We need inputs as to how to move ahead from here ? <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10765) <!-- Reviewable:end -->
|
|
highfive
commented
May 2, 2016
|
highfive
commented
May 3, 2016
|
New code was committed to pull request. |
|
|
|
Thank you for working on this! I have noted this work in #11420 for anyone who takes on this effort. This work doesn't make sense to merge as-is, though, since the actual hierarchical relationships between the various DOM types aren't implemented. |
Ishabobra commentedApr 20, 2016
We need inputs as to how to move ahead from here ?
This change is